Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
10 changes: 10 additions & 0 deletions packages/tx/src/eip1559Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,16 @@ export default class FeeMarketEIP1559Transaction extends BaseTransaction<FeeMark
throw new Error(msg)
}

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

const freeze = opts?.freeze ?? true
if (freeze) {
Object.freeze(this)
Expand Down
10 changes: 10 additions & 0 deletions packages/tx/src/eip2930Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,16 @@ export default class AccessListEIP2930Transaction extends BaseTransaction<Access
throw new Error(msg)
}

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

const freeze = opts?.freeze ?? true
if (freeze) {
Object.freeze(this)
Expand Down
10 changes: 10 additions & 0 deletions packages/tx/src/legacyTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,16 @@ 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

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

const freeze = opts?.freeze ?? true
if (freeze) {
Object.freeze(this)
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