Skip to content
This repository has been archived by the owner on Dec 5, 2021. It is now read-only.

Commit

Permalink
feat: fees v2 (ethereum-optimism#976)
Browse files Browse the repository at this point in the history
* l2 geth: new fee logic

* l2 geth: migrate to fees package

* core-utils: new fee scheme

* chore: add changeset

* l2geth: delete dead code

* integration-tests: fix typo

* integration-tests: fixes

* fees: use fee scalar

* lint: fix

* rollup: correct gas payment comparison

* fix(integration-tests): do not hardcode gas price

* core-utils: update with new scheme

* l2geth: refactor rollup oracle

* l2geth: clean up DoEstimateGas

* l2geth: implement latest scheme

* tests: fix up

* lint: fix

* l2geth: better sycn service test

* optimism: rename to TxGasLimit

* fee: fix docstring

* tests: fix

* variables: rename

* l2geth: prevent users from sending txs with too high of a fee

* integration-tests: fix import

* integration-tests: fix type

* integration-tests: fix gas limits

* lint: fix

* l2geth: log error

Co-authored-by: Georgios Konstantopoulos <[email protected]>
  • Loading branch information
2 people authored and InoMurko committed May 31, 2021
1 parent 28dbe4b commit 5d8c029
Show file tree
Hide file tree
Showing 17 changed files with 402 additions and 500 deletions.
6 changes: 6 additions & 0 deletions .changeset/fuzzy-dogs-share.md
Original file line number Diff line number Diff line change
@@ -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
6 changes: 5 additions & 1 deletion integration-tests/test/erc20.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Contract, ContractFactory, Wallet } from 'ethers'
import { ethers } from 'hardhat'
import { TxGasLimit, TxGasPrice } from '@eth-optimism/core-utils'
import chai, { expect } from 'chai'
import { GWEI } from './shared/utils'
import { OptimismEnv } from './shared/env'
Expand Down Expand Up @@ -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(TxGasPrice)
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)
Expand Down
7 changes: 3 additions & 4 deletions integration-tests/test/fee-payment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
})

Expand All @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions integration-tests/test/native-eth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(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(0x207ad91a77b4))
expect(gas).to.be.deep.eq(BigNumber.from(61400489396))
})
})

Expand Down
48 changes: 32 additions & 16 deletions integration-tests/test/rpc.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {
injectL2Context,
L2GasLimit,
roundL1GasPrice,
TxGasLimit,
TxGasPrice,
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 {
Expand Down Expand Up @@ -130,11 +131,25 @@ describe('Basic RPC tests', () => {
const tx = {
...DEFAULT_TRANSACTION,
gasLimit: 1,
gasPrice: 1,
gasPrice: TxGasPrice,
}
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 = 33600000000001 and tx.gasPrice = 1'
`fee too low: ${fee}, use at least tx.gasLimit = ${gasLimit} and tx.gasPrice = ${TxGasPrice.toString()}`
)
})

it('should reject a transaction with an incorrect gas price', async () => {
const tx = {
...DEFAULT_TRANSACTION,
gasLimit: 1,
gasPrice: TxGasPrice.sub(1),
}

await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith(
`tx.gasPrice must be ${TxGasPrice.toString()}`
)
})

Expand Down Expand Up @@ -198,7 +213,7 @@ describe('Basic RPC tests', () => {
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)
Expand All @@ -221,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)
Expand Down Expand Up @@ -311,12 +326,14 @@ 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(
TxGasPrice.toNumber()
)
})
})

describe('eth_estimateGas (returns the fee)', () => {
describe('eth_estimateGas (returns the scaled fee)', () => {
it('gas estimation is deterministic', async () => {
let lastEstimate: BigNumber
for (let i = 0; i < 10; i++) {
Expand All @@ -338,7 +355,7 @@ describe('Basic RPC tests', () => {
to: DEFAULT_TRANSACTION.to,
value: 0,
})
expect(estimate).to.be.eq(33600000119751)
expect(estimate).to.be.eq(0x0dce9004c7)
})

it('should return a gas estimate that grows with the size of data', async () => {
Expand All @@ -349,7 +366,6 @@ describe('Basic RPC tests', () => {
for (const data of dataLen) {
const tx = {
to: '0x' + '1234'.repeat(10),
gasPrice: '0x1',
value: '0x0',
data: '0x' + '00'.repeat(data),
from: '0x' + '1234'.repeat(10),
Expand All @@ -359,16 +375,16 @@ 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 l2GasPrice = BigNumber.from(1)
const expected = L2GasLimit.encode({
const expected = TxGasLimit.encode({
data: tx.data,
l1GasPrice: roundL1GasPrice(l1GasPrice),
l1GasPrice,
l2GasLimit: BigNumber.from(l2Gaslimit),
l2GasPrice,
})
Expand Down
126 changes: 0 additions & 126 deletions l2geth/core/rollup_fee.go

This file was deleted.

Loading

0 comments on commit 5d8c029

Please sign in to comment.