-
Notifications
You must be signed in to change notification settings - Fork 773
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, Common: implement eip-3540 (EOF1) #1719
Changes from all commits
66a1054
a162fec
e214e1b
b9a9a29
eb3e982
3cdb742
222e496
f7ab311
080a912
583fa21
3b5b208
3c560e8
d77e3e0
a650bd0
6f92ceb
fc5ead8
f034ba1
7815b71
c16877a
57a3f3a
ce2f314
8b10866
ef45dea
88afc65
38a3141
f27bc01
605e576
1114476
3fb6bab
80933d2
9d028f0
6282e89
9fea5bb
8f2d17b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
{ | ||
"name": "EIP-3540", | ||
"number": 3540, | ||
"comment": "EVM Object Format (EOF) v1", | ||
"url": "https://eips.ethereum.org/EIPS/eip-3540", | ||
"status": "Review", | ||
"minimumHardfork": "london", | ||
"requiredEIPs": [ | ||
3541 | ||
], | ||
"gasConfig": {}, | ||
"gasPrices": {}, | ||
"vm": {}, | ||
"pow": {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
{ | ||
"name": "EIP-3670", | ||
"number": 3670, | ||
"comment": "EOF - Code Validation", | ||
"url": "https://eips.ethereum.org/EIPS/eip-3670", | ||
"status": "Review", | ||
"minimumHardfork": "london", | ||
"requiredEIPs": [ | ||
3540 | ||
], | ||
"gasConfig": {}, | ||
"gasPrices": {}, | ||
"vm": {}, | ||
"pow": {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import Common from '@ethereumjs/common' | ||
import { BN, keccak256, setLengthRight, setLengthLeft } from 'ethereumjs-util' | ||
import { handlers } from '.' | ||
import { ERROR, VmError } from './../../exceptions' | ||
import { RunState } from './../interpreter' | ||
|
||
|
@@ -261,3 +262,93 @@ export function updateSstoreGas( | |
return new BN(common.param('gasPrices', 'sstoreSet')) | ||
} | ||
} | ||
|
||
/** | ||
* | ||
* @param container A `Buffer` containing bytecode to be checked for EOF1 compliance | ||
* @returns an object containing the size of the code section and data sections for a valid | ||
* EOF1 container or else undefined if `container` is not valid EOF1 bytecode | ||
* | ||
* Note: See https://eips.ethereum.org/EIPS/eip-3540 for further details | ||
*/ | ||
export const eof1CodeAnalysis = (container: Buffer) => { | ||
const magic = 0x00 | ||
const version = 0x01 | ||
const secCode = 0x01 | ||
const secData = 0x02 | ||
const secTerminator = 0x00 | ||
let computedContainerSize = 0 | ||
const sectionSizes = { | ||
code: 0, | ||
data: 0, | ||
} | ||
if (container[1] !== magic || container[2] !== version) | ||
// Bytecode does not contain EOF1 "magic" or version number in expected positions | ||
return | ||
|
||
if ( | ||
// EOF1 bytecode must be more than 7 bytes long for EOF1 header plus code section (but no data section) | ||
container.length > 7 && | ||
// EOF1 code section indicator | ||
container[3] === secCode && | ||
// EOF1 header terminator | ||
container[6] === secTerminator | ||
) { | ||
sectionSizes.code = (container[4] << 8) | container[5] | ||
// Calculate expected length of EOF1 container based on code section | ||
computedContainerSize = 7 + sectionSizes.code | ||
// EOF1 code section must be at least 1 byte long | ||
if (sectionSizes.code < 1) return | ||
} else if ( | ||
// EOF1 container must be more than 10 bytes long if data section is included | ||
container.length > 10 && | ||
// EOF1 code section indicator | ||
container[3] === secCode && | ||
// EOF1 data section indicator | ||
container[6] === secData && | ||
// EOF1 header terminator | ||
container[9] === secTerminator | ||
) { | ||
sectionSizes.code = (container[4] << 8) | container[5] | ||
sectionSizes.data = (container[7] << 8) | container[8] | ||
// Calculate expected length of EOF1 container based on code and data sections | ||
computedContainerSize = 10 + sectionSizes.code + sectionSizes.data | ||
// Code & Data sizes cannot be 0 | ||
if (sectionSizes.code < 1 || sectionSizes.data < 1) return | ||
} | ||
if (container.length !== computedContainerSize) { | ||
// Computed container length based on section details does not match length of actual bytecode | ||
return | ||
} | ||
return sectionSizes | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is checking on so many conditions in such a condensed form that it is extremely hard to review and generally follow. Can we have this dramatically [TM] more commented, so basically to a comment with a condition on more or less every line of code? Guess that would make things easier to grasp. Also: one technique @ryanio often applies and I guess which would help here as well is to instead of checking for the positive condition ( |
||
|
||
export const eof1ValidOpcodes = (code: Buffer) => { | ||
// EIP-3670 - validate all opcodes | ||
const opcodes = new Set(handlers.keys()) | ||
opcodes.add(0xfe) // Add INVALID opcode to set | ||
|
||
let x = 0 | ||
while (x < code.length) { | ||
const opcode = code[x] | ||
x++ | ||
if (!opcodes.has(opcode)) { | ||
// No invalid/undefined opcodes | ||
return false | ||
} | ||
if (opcode >= 0x60 && opcode <= 0x7f) { | ||
// Skip data block following push | ||
x += opcode - 0x5f | ||
if (x > code.length - 1) { | ||
// Push blocks must not exceed end of code section | ||
return false | ||
} | ||
} | ||
} | ||
const terminatingOpcodes = new Set([0x00, 0xf3, 0xfd, 0xfe, 0xff]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh wow, missed this 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, me too. But writing those new tests you suggested helped me catch it 🎆 |
||
// Per EIP-3670, the final opcode of a code section must be STOP, RETURN, REVERT, INVALID, or SELFDESTRUCT | ||
if (!terminatingOpcodes.has(code[code.length - 1])) { | ||
return false | ||
} | ||
return true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is from the
EIP-3670
spec:Might it be that you are doing validation only on the code and not the initcode? Or am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is checking the code returned by the create contract to verify its valid . The initcode is checked when it is run through the
runInterpreter
function call.