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

VM, Tx, Common: Implement EIP3860 "limit and meter initcode" #1619

Merged
merged 14 commits into from
Mar 9, 2022
Merged
23 changes: 23 additions & 0 deletions packages/common/src/eips/3860.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"name": "EIP-3860",
"number": 3860,
"comment": "Limit and meter initcode",
"url": "https://eips.ethereum.org/EIPS/eip-3860",
"status": "Review",
"minimumHardfork": "spuriousDragon",
"requiredEIPs": [],
"gasConfig": {},
"gasPrices": {
"initCodeWordCost": {
"v": 2,
"d": "Gas to pay for each word (32 bytes) of initcode when creating a contract"
}
},
"vm": {
"maxInitCodeSize": {
"v": 49152,
"d": "Maximum length of initialization code when creating a contract"
}
},
"pow": {}
}
1 change: 1 addition & 0 deletions packages/common/src/eips/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const EIPs: eipsType = {
3607: require('./3607.json'),
3675: require('./3675.json'),
3855: require('./3855.json'),
3860: require('./3860.json'),
4345: require('./4345.json'),
4399: require('./4399.json'),
}
13 changes: 11 additions & 2 deletions packages/tx/src/baseTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,21 @@ export abstract class BaseTransaction<TransactionObject> {
const txDataZero = this.common.param('gasPrices', 'txDataZero')
const txDataNonZero = this.common.param('gasPrices', 'txDataNonZero')

let cost = 0
let cost: number | BN = 0
for (let i = 0; i < this.data.length; i++) {
this.data[i] === 0 ? (cost += txDataZero) : (cost += txDataNonZero)
}

return new BN(cost)
cost = new BN(cost)
if ((this.to === undefined || this.to === null) && this.common.isActivatedEIP(3860)) {
const dataLength = Math.ceil(this.data.length / 32)
const initCodeCost = new BN(this.common.param('gasPrices', 'initCodeWordCost')).imuln(
dataLength
)
cost.iadd(initCodeCost)
}

return cost
}

/**
Expand Down
6 changes: 5 additions & 1 deletion packages/tx/src/eip1559Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
N_DIV_2,
TxOptions,
} from './types'
import { AccessLists } from './util'
import { AccessLists, checkMaxInitCodeSize } from './util'

const TRANSACTION_TYPE = 2
const TRANSACTION_TYPE_BUFFER = Buffer.from(TRANSACTION_TYPE.toString(16).padStart(2, '0'), 'hex')
Expand Down Expand Up @@ -235,6 +235,10 @@ export default class FeeMarketEIP1559Transaction extends BaseTransaction<FeeMark
throw new Error(msg)
}

if (this.common.isActivatedEIP(3860)) {
checkMaxInitCodeSize(this.common, this.data.length)
}

const freeze = opts?.freeze ?? true
if (freeze) {
Object.freeze(this)
Expand Down
5 changes: 4 additions & 1 deletion packages/tx/src/eip2930Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
N_DIV_2,
} from './types'

import { AccessLists } from './util'
import { AccessLists, checkMaxInitCodeSize } from './util'

const TRANSACTION_TYPE = 1
const TRANSACTION_TYPE_BUFFER = Buffer.from(TRANSACTION_TYPE.toString(16).padStart(2, '0'), 'hex')
Expand Down Expand Up @@ -212,6 +212,9 @@ export default class AccessListEIP2930Transaction extends BaseTransaction<Access
throw new Error(msg)
}

if (this.common.isActivatedEIP(3860)) {
checkMaxInitCodeSize(this.common, this.data.length)
}
const freeze = opts?.freeze ?? true
if (freeze) {
Object.freeze(this)
Expand Down
5 changes: 5 additions & 0 deletions packages/tx/src/legacyTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { TxOptions, TxData, JsonTx, N_DIV_2, TxValuesArray, Capability } from './types'
import { BaseTransaction } from './baseTransaction'
import Common from '@ethereumjs/common'
import { checkMaxInitCodeSize } from './util'

const TRANSACTION_TYPE = 0

Expand Down Expand Up @@ -135,6 +136,10 @@ export default class Transaction extends BaseTransaction<Transaction> {
}
}

if (this.common.isActivatedEIP(3860)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block is in all three transaction types, the reason is that we do not have access to common in baseTransaction. Adding it in baseTransaction is somewhat hard as especially legacyTransaction has some extra logic to extract chainId.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm. i don't like how we have to keep duplicating code across the txs classes like this. but thanks for the reasoning why.

would be much better if we always write the code just once. what if we move it to a method inutil.ts called e.g. checkMaxInitCodeSize(), then we can just keep the if isActivatedEIP(3860) and call the method inside.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than that, PR looks great! I'm not sure why the errors don't have any coverage on them though (I'm seeing codecov warnings), i assume the added TransactionTests should cover that. if not maybe we should add a few explicit tests for coverage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, great suggestion moving it to util! I also did not like it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed this

checkMaxInitCodeSize(this.common, this.data.length)
}

const freeze = opts?.freeze ?? true
if (freeze) {
Object.freeze(this)
Expand Down
11 changes: 11 additions & 0 deletions packages/tx/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,17 @@ import Common from '@ethereumjs/common'
import { bufferToHex, setLengthLeft, toBuffer } from 'ethereumjs-util'
import { AccessList, AccessListBuffer, AccessListItem, isAccessList } from './types'

export function checkMaxInitCodeSize(common: Common, length: number) {
if (length > common.param('vm', 'maxInitCodeSize')) {
throw new Error(
`the initcode size of this transaction is too large: it is ${length} while the max is ${common.param(
'vm',
'maxInitCodeSize'
)}`
)
}
}

export class AccessLists {
public static getAccessListData(accessList: AccessListBuffer | AccessList) {
let AccessListJSON
Expand Down
13 changes: 13 additions & 0 deletions packages/tx/test/transactionRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const argv = minimist(process.argv.slice(2))
const file: string | undefined = argv.file

const forkNames: ForkName[] = [
'London+3860',
'London',
'Berlin',
'Istanbul',
Expand All @@ -23,6 +24,7 @@ const forkNames: ForkName[] = [
]

const forkNameMap: ForkNamesMap = {
'London+3860': 'london',
London: 'london',
Berlin: 'berlin',
Istanbul: 'istanbul',
Expand All @@ -35,6 +37,10 @@ const forkNameMap: ForkNamesMap = {
Homestead: 'homestead',
}

const EIPs: any = {
'London+3860': [3860],
}

tape('TransactionTests', async (t) => {
const fileFilterRegex = file ? new RegExp(file + '[^\\w]') : undefined
await getTests(
Expand All @@ -46,13 +52,20 @@ tape('TransactionTests', async (t) => {
) => {
t.test(testName, (st) => {
for (const forkName of forkNames) {
if (testData.result[forkName] === undefined) {
continue
}
const forkTestData = testData.result[forkName]
const shouldBeInvalid = !!forkTestData.exception

try {
const rawTx = toBuffer(testData.txbytes)
const hardfork = forkNameMap[forkName]
const common = new Common({ chain: 1, hardfork })
const activateEIPs = EIPs[forkName]
if (activateEIPs) {
common.setEIPs(activateEIPs)
}
const tx = Transaction.fromSerializedTx(rawTx, { common })
const sender = tx.getSenderAddress().toString()
const hash = tx.hash().toString('hex')
Expand Down
1 change: 1 addition & 0 deletions packages/tx/test/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export type ForkName =
| 'London+3860'
| 'London'
| 'Berlin'
| 'Istanbul'
Expand Down
7 changes: 7 additions & 0 deletions packages/vm/src/evm/eei.ts
Original file line number Diff line number Diff line change
Expand Up @@ -600,9 +600,16 @@ export default class EEI {
if (this._env.contract.nonce.gte(MAX_UINT64)) {
return new BN(0)
}

this._env.contract.nonce.iaddn(1)
await this._state.putAccount(this._env.address, this._env.contract)

if (this._common.isActivatedEIP(3860)) {
if (msg.data.length > this._common.param('vm', 'maxInitCodeSize')) {
return new BN(0)
}
}

const results = await this._evm.executeMessage(msg)

if (results.execResult.logs) {
Expand Down
14 changes: 14 additions & 0 deletions packages/vm/src/evm/evm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,20 @@ export default class EVM {
// Reduce tx value from sender
await this._reduceSenderBalance(account, message)

if (this._vm._common.isActivatedEIP(3860)) {
if (message.data.length > this._vm._common.param('vm', 'maxInitCodeSize')) {
return {
gasUsed: message.gasLimit,
createdAddress: message.to,
execResult: {
returnValue: Buffer.alloc(0),
exceptionError: new VmError(ERROR.INITCODE_SIZE_VIOLATION),
gasUsed: message.gasLimit,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I read the EIP, it says that if a "create transaction exceeds MAX_INITCODE_SIZE, transaction is invalid." Does that mean that we do an exceptional abort and consume all gas or that the transaction is just rejected and not included in the block?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and added a new EIP-3860 test file to the api/EIPs section and added a real basic test to trigger this error so we can bump test coverage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the latter, I remember having the same question and it is answered somewhere, but I cannot find it again 😅 So, the block is invalid if you include such transaction and nodes will reject that broadcasted block.

},
}
}
}

message.code = message.data
message.data = Buffer.alloc(0)
message.to = await this._generateAddress(message)
Expand Down
17 changes: 17 additions & 0 deletions packages/vm/src/evm/opcodes/gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ export const dynamicGasHandlers: Map<number, AsyncDynamicGasHandler> = new Map<

gas.iadd(subMemUsage(runState, offset, length, common))

if (common.isActivatedEIP(3860)) {
// Meter initcode
const initCodeCost = new BN(common.param('gasPrices', 'initCodeWordCost')).imul(
length.addn(31).divn(32)
)
gas.iadd(initCodeCost)
}

let gasLimit = new BN(runState.eei.getGasLeft().isub(gas))
gasLimit = maxCallGas(gasLimit.clone(), gasLimit.clone(), runState, common)

Expand Down Expand Up @@ -419,6 +427,15 @@ export const dynamicGasHandlers: Map<number, AsyncDynamicGasHandler> = new Map<
}

gas.iadd(new BN(common.param('gasPrices', 'sha3Word')).imul(divCeil(length, new BN(32))))

if (common.isActivatedEIP(3860)) {
// Meter initcode
const initCodeCost = new BN(common.param('gasPrices', 'initCodeWordCost')).imul(
length.addn(31).divn(32)
)
gas.iadd(initCodeCost)
}

let gasLimit = new BN(runState.eei.getGasLeft().isub(gas))
gasLimit = maxCallGas(gasLimit.clone(), gasLimit.clone(), runState, common) // CREATE2 is only available after TangerineWhistle (Constantinople introduced this opcode)
runState.messageGasLimit = gasLimit
Expand Down
1 change: 1 addition & 0 deletions packages/vm/src/exceptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export enum ERROR {
INVALID_RETURNSUB = 'invalid RETURNSUB',
INVALID_JUMPSUB = 'invalid JUMPSUB',
INVALID_BYTECODE_RESULT = 'invalid bytecode deployed',
INITCODE_SIZE_VIOLATION = 'initcode exceeds max initcode size',

// BLS errors
BLS_12_381_INVALID_INPUT_LENGTH = 'invalid input length',
Expand Down
5 changes: 4 additions & 1 deletion packages/vm/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export interface VMOpts {
* - [EIP-3529](https://eips.ethereum.org/EIPS/eip-3529) - Reduction in refunds
* - [EIP-3541](https://eips.ethereum.org/EIPS/eip-3541) - Reject new contracts starting with the 0xEF byte
* - [EIP-3855](https://eips.ethereum.org/EIPS/eip-3855) - PUSH0 instruction
* - [EIP-3860](https://eips.ethereum.org/EIPS/eip-3860) - Limit and meter initcode
*
* *Annotations:*
*
Expand Down Expand Up @@ -195,7 +196,9 @@ export default class VM extends AsyncEventEmitter {

if (opts.common) {
// Supported EIPs
const supportedEIPs = [1559, 2315, 2537, 2565, 2718, 2929, 2930, 3198, 3529, 3541, 3607, 3855]
const supportedEIPs = [
1559, 2315, 2537, 2565, 2718, 2929, 2930, 3198, 3529, 3541, 3607, 3855, 3860,
]
for (const eip of opts.common.eips()) {
if (!supportedEIPs.includes(eip)) {
throw new Error(`EIP-${eip} is not supported by the VM`)
Expand Down
40 changes: 40 additions & 0 deletions packages/vm/tests/api/EIPs/eip-3860.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import tape from 'tape'
import VM from '../../../src'
import Common, { Chain, Hardfork } from '@ethereumjs/common'
import { FeeMarketEIP1559Transaction } from '@ethereumjs/tx'
import { Address, BN, privateToAddress } from 'ethereumjs-util'
const pkey = Buffer.from('20'.repeat(32), 'hex')
const GWEI = new BN('1000000000')
const sender = new Address(privateToAddress(pkey))

tape('EIP 3860 tests', (t) => {
const common = new Common({
chain: Chain.Mainnet,
hardfork: Hardfork.London,
eips: [3860],
})

t.test('EIP-3860 tests', async (st) => {
const vm = new VM({ common })
const account = await vm.stateManager.getAccount(sender)
const balance = GWEI.muln(21000).muln(10000000)
account.balance = balance
await vm.stateManager.putAccount(sender, account)

const buffer = Buffer.allocUnsafe(1000000).fill(0x60)
console.log(common.isActivatedEIP(3860))
const tx = FeeMarketEIP1559Transaction.fromTxData({
data:
'0x7F6000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000060005260206000F3' +
buffer.toString('hex'),
gasLimit: 100000000000,
maxFeePerGas: 7,
nonce: 0,
}).sign(pkey)
const result = await vm.runTx({ tx })
st.ok(
(result.execResult.exceptionError?.error as string) === 'initcode exceeds max initcode size',
'initcode exceeds max size'
)
})
})