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.

Sign up for GitHub

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

Subdivide runState, refactor loop #498

Merged
merged 11 commits into from
Apr 25, 2019
Merged

Subdivide runState, refactor loop #498

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d037219
Break runState into runState, result, env
s1na Apr 23, 2019
b7139e0
Drop emitFreeLogs flag, mv static from runState to env
s1na Apr 23, 2019
3a94f2d
Mv depth to env, throw on pc out of range
s1na Apr 23, 2019
becd29b
Add interpreter runPrecompile to replace runJit
s1na Apr 23, 2019
15a1499
Pass message & txContext directly to loop.run
s1na Apr 23, 2019
e205a44
evm: make runState an attribute of loop, mv lastReturned to eei
s1na Apr 23, 2019
9339bb0
evm: fix isAccountEmpty
s1na Apr 23, 2019
446fbdf
evm: mv parseVmResults logic to loop.run
s1na Apr 23, 2019
e27ac52
Minor improvements
s1na Apr 24, 2019
5c721b5
Fix linting errors
s1na Apr 24, 2019
51b8d6b
Fix typo
s1na Apr 24, 2019
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
113 changes: 62 additions & 51 deletions lib/evm/eei.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,17 @@ const { VmError, ERROR } = require('../exceptions')

export default class EEI {
_env: any
_runState: any
_result: any
_state: PStateManager
_interpreter: any

constructor (env: any) {
constructor (env: any, runState: any, result: any, state: PStateManager, interpreter: any) {
Copy link
Member

Choose a reason for hiding this comment

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

Much cleaner separation like this with reduced runState + env and explicit results.

Not sure if interpreter should be passed to the EEI, but it's at least better to have this passed explicit/separate then being hidden in the environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interpreter is needed for calls and creates. Not sure how to work around it...

Copy link
Member

Choose a reason for hiding this comment

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

Might these kind of logic rather belong to the Interpreter itself? We can keep for now and keep this in discussion though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm thinking along the same lines. I think it makes sense to move most of the call/create logic to Interpreter. One downside would be however that the logic would be further away from the opcode handlers.

this._env = env
this._state = new PStateManager(this._env.stateManager)
this._runState = runState
this._result = result
this._state = state
this._interpreter = interpreter
}

/**
Expand All @@ -20,13 +26,17 @@ export default class EEI {
* @throws if out of gas
*/
useGas (amount: BN): void {
this._env.gasLeft.isub(amount)
if (this._env.gasLeft.ltn(0)) {
this._env.gasLeft = new BN(0)
this._runState.gasLeft.isub(amount)
if (this._runState.gasLeft.ltn(0)) {
this._runState.gasLeft = new BN(0)
trap(ERROR.OUT_OF_GAS)
}
}

refundGas (amount: BN): void {
this._result.gasRefund.iaddn(amount)
}

/**
* Returns address of currently executing account.
* @returns {Buffer}
Expand Down Expand Up @@ -137,7 +147,7 @@ export default class EEI {
* @returns {BN}
*/
getReturnDataSize (): BN {
return new BN(this._env.lastReturned.length)
return new BN(this._runState.lastReturned.length)
}

/**
Expand All @@ -147,7 +157,7 @@ export default class EEI {
* @returns {Buffer}
*/
getReturnData (): Buffer {
return this._env.lastReturned
return this._runState.lastReturned
}

/**
Expand Down Expand Up @@ -242,15 +252,15 @@ export default class EEI {
* @returns {BN}
*/
getGasLeft (): BN {
return new BN(this._env.gasLeft)
return new BN(this._runState.gasLeft)
}

/**
* Set the returning output data for the execution.
* @param {Buffer} returnData - Output data to return
*/
finish (returnData: Buffer): void {
this._env.returnValue = returnData
this._result.returnValue = returnData
}

/**
Expand All @@ -259,7 +269,7 @@ export default class EEI {
* @param {Buffer} returnData - Output data to return
*/
revert (returnData: Buffer): void {
this._env.returnValue = returnData
this._result.returnValue = returnData
trap(ERROR.REVERT)
}

Expand All @@ -274,21 +284,13 @@ export default class EEI {
}

async _selfDestruct (toAddress: Buffer): Promise<void> {
// TODO: Determine if gas consumption & refund should happen in EEI or opFn
if ((new BN(this._env.contract.balance)).gtn(0)) {
const empty = await this._state.accountIsEmpty(toAddress)
if (empty) {
this.useGas(new BN(this._env._common.param('gasPrices', 'callNewAccount')))
}
}

// only add to refund if this is the first selfdestruct for the address
if (!this._env.selfdestruct[this._env.address.toString('hex')]) {
this._env.gasRefund = this._env.gasRefund.addn(this._env._common.param('gasPrices', 'selfdestructRefund'))
if (!this._result.selfdestruct[this._env.address.toString('hex')]) {
this._result.gasRefund = this._result.gasRefund.addn(this._runState._common.param('gasPrices', 'selfdestructRefund'))
}

this._env.selfdestruct[this._env.address.toString('hex')] = toAddress
this._env.stopped = true
this._result.selfdestruct[this._env.address.toString('hex')] = toAddress
this._runState.stopped = true

// Add to beneficiary balance
const toAccount = await this._state.getAccount(toAddress)
Expand Down Expand Up @@ -323,7 +325,7 @@ export default class EEI {

// add data
log.push(data)
this._env.logs.push(log)
this._result.logs.push(log)
}

/**
Expand All @@ -340,8 +342,8 @@ export default class EEI {
to: address,
value: value,
data: data,
isStatic: this._env.static,
depth: this._env.depth + 1
isStatic: this._runState.static,
depth: this._runState.depth + 1
})

return this._baseCall(msg)
Expand All @@ -362,8 +364,8 @@ export default class EEI {
codeAddress: address,
value: value,
data: data,
isStatic: this._env.static,
depth: this._env.depth + 1
isStatic: this._runState.static,
depth: this._runState.depth + 1
})

return this._baseCall(msg)
Expand All @@ -386,7 +388,7 @@ export default class EEI {
value: value,
data: data,
isStatic: true,
depth: this._env.depth + 1
depth: this._runState.depth + 1
})

return this._baseCall(msg)
Expand All @@ -408,48 +410,47 @@ export default class EEI {
codeAddress: address,
value: value,
data: data,
isStatic: this._env.static,
isStatic: this._runState.static,
delegatecall: true,
depth: this._env.depth + 1
depth: this._runState.depth + 1
})

return this._baseCall(msg)
}

async _baseCall (msg: Message): Promise<BN> {
const selfdestruct = Object.assign({}, this._env.selfdestruct)
const selfdestruct = Object.assign({}, this._result.selfdestruct)
msg.selfdestruct = selfdestruct

// empty the return data buffer
this._env.lastReturned = Buffer.alloc(0)
this._runState.lastReturned = Buffer.alloc(0)

// Check if account has enough ether and max depth not exceeded
if (this._env.depth >= this._env._common.param('vm', 'stackLimit') || (msg.delegatecall !== true && new BN(this._env.contract.balance).lt(msg.value))) {
if (this._runState.depth >= this._runState._common.param('vm', 'stackLimit') || (msg.delegatecall !== true && new BN(this._env.contract.balance).lt(msg.value))) {
return new BN(0)
}

const results = await this._env.interpreter.executeMessage(msg)
const results = await this._interpreter.executeMessage(msg)

// concat the runState.logs
if (results.vm.logs) {
this._env.logs = this._env.logs.concat(results.vm.logs)
this._result.logs = this._result.logs.concat(results.vm.logs)
}

// add gasRefund
if (results.vm.gasRefund) {
this._env.gasRefund = this._env.gasRefund.add(results.vm.gasRefund)
this._result.gasRefund = this._result.gasRefund.add(results.vm.gasRefund)
}

// this should always be safe
this._env.gasLeft.isub(results.gasUsed)
this.useGas(results.gasUsed)

// Set return value
if (results.vm.return && (!results.vm.exceptionError || results.vm.exceptionError.error === ERROR.REVERT)) {
this._env.lastReturned = results.vm.return
this._runState.lastReturned = results.vm.return
}

if (!results.vm.exceptionError) {
Object.assign(this._env.selfdestruct, selfdestruct)
Object.assign(this._result.selfdestruct, selfdestruct)
// update stateRoot on current contract
const account = await this._state.getAccount(this._env.address)
this._env.contract = account
Expand All @@ -465,50 +466,49 @@ export default class EEI {
* @param {Buffer} data
*/
async create (gasLimit: BN, value: BN, data: Buffer, salt: Buffer | null = null): Promise<BN> {
const selfdestruct = Object.assign({}, this._env.selfdestruct)
const selfdestruct = Object.assign({}, this._result.selfdestruct)
const msg = new Message({
caller: this._env.address,
gasLimit: gasLimit,
value: value,
data: data,
salt: salt,
depth: this._env.depth + 1,
depth: this._runState.depth + 1,
selfdestruct: selfdestruct
})

// empty the return data buffer
this._env.lastReturned = Buffer.alloc(0)
this._runState.lastReturned = Buffer.alloc(0)

// Check if account has enough ether and max depth not exceeded
if (this._env.depth >= this._env._common.param('vm', 'stackLimit') || (msg.delegatecall !== true && new BN(this._env.contract.balance).lt(msg.value))) {
if (this._runState.depth >= this._runState._common.param('vm', 'stackLimit') || (msg.delegatecall !== true && new BN(this._env.contract.balance).lt(msg.value))) {
return new BN(0)
}

this._env.contract.nonce = new BN(this._env.contract.nonce).addn(1)
await this._state.putAccount(this._env.address, this._env.contract)

const results = await this._env.interpreter.executeMessage(msg)
const results = await this._interpreter.executeMessage(msg)

// concat the runState.logs
if (results.vm.logs) {
this._env.logs = this._env.logs.concat(results.vm.logs)
this._result.logs = this._result.logs.concat(results.vm.logs)
}

// add gasRefund
if (results.vm.gasRefund) {
this._env.gasRefund = this._env.gasRefund.add(results.vm.gasRefund)
this._result.gasRefund = this._result.gasRefund.add(results.vm.gasRefund)
}

// this should always be safe
this._env.gasLeft.isub(results.gasUsed)
this.useGas(results.gasUsed)

// Set return buffer in case revert happened
if (results.vm.exceptionError && results.vm.exceptionError.error === ERROR.REVERT) {
this._env.lastReturned = results.vm.return
this._runState.lastReturned = results.vm.return
}

if (!results.vm.exceptionError) {
Object.assign(this._env.selfdestruct, selfdestruct)
Object.assign(this._result.selfdestruct, selfdestruct)
// update stateRoot on current contract
const account = await this._state.getAccount(this._env.address)
this._env.contract = account
Expand Down Expand Up @@ -537,6 +537,16 @@ export default class EEI {
async create2 (gasLimit: BN, value: BN, data: Buffer, salt: Buffer): Promise<BN> {
return this.create(gasLimit, value, data, salt)
}

/**
* Returns true if account is empty (according to EIP-161).
* @param address - Address of account
*/
async isAccountEmpty (address: BN): Promise<boolean> {
const addressBuf = addressToBuffer(address)
const account = await this._state.getAccount(addressBuf)
return account.isEmpty()
}
}

function trap (err: string) {
Expand All @@ -545,5 +555,6 @@ function trap (err: string) {

const MASK_160 = new BN(1).shln(160).subn(1)
function addressToBuffer (address: BN) {
if (Buffer.isBuffer(address)) return address
return address.and(MASK_160).toArrayLike(Buffer, 'be', 20)
}
Loading