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

implement Hofmann structure #169

Merged
merged 12 commits into from
Jan 13, 2020
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
120 changes: 116 additions & 4 deletions contracts/EVMRuntime.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,24 @@ import { EVMMemory } from "./EVMMemory.slb";
import { EVMStack } from "./EVMStack.slb";
import { EVMUtils } from "./EVMUtils.slb";
import { EVMCode } from "./EVMCode.slb";
import { EVMTokenBag } from "./EVMTokenBag.slb";


contract EVMRuntime is EVMConstants {
using EVMMemory for EVMMemory.Memory;
using EVMStack for EVMStack.Stack;
using EVMCode for EVMCode.Code;
using EVMTokenBag for EVMTokenBag.TokenBag;

// bridge has to defreagment the tokens
Copy link
Member

Choose a reason for hiding this comment

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

minor detail, but it is not actually the Bridge, but the Enforcer that will have to do these things. see here: https://docs.google.com/drawings/d/1WXcAvTEpXDGeL68MfWOP20mERht8rzqfSzdiFtYZeSc
in that diagram the bridge takes the client contract role.
same in this diagram: https://docs.google.com/drawings/d/1vlRmrHJd-eozM9e34pgCEnm8Iuq9bhqVnopOva7Tfz4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well the bridge should do that and enforcer just passes onwards i guess? Details of plasma should stay out of the EVM-enforcer as much as possible.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, i think we need some intermediate contract here. the bridge only understand transactions, but not runtime.

// bridge has to check approvals
// bridge has to convert color to address
// we are assuming all token calls cost 0 for now
// find correct funcSigs

function getSig(bytes memory _msgData) internal pure returns (bytes4) {
return bytes4(_msgData[3]) >> 24 | bytes4(_msgData[2]) >> 16 | bytes4(_msgData[1]) >> 8 | bytes4(_msgData[0]);
}

// what we do not track (not complete list)
// call depth: as we do not support stateful things like call to other contracts
Expand All @@ -31,7 +43,8 @@ contract EVMRuntime is EVMConstants {
EVMCode.Code code;
EVMMemory.Memory mem;
EVMStack.Stack stack;

EVMTokenBag.TokenBag tokenBag;

uint256 blockNumber;
uint256 blockHash;
uint256 blockTime;
Expand Down Expand Up @@ -1551,8 +1564,76 @@ contract EVMRuntime is EVMConstants {
state.errno = ERROR_INSTRUCTION_NOT_SUPPORTED;
}

struct StackForCall {
uint gas;
address target;
uint value;
uint inOffset;
uint inSize;
uint retOffset;
uint retSize;
}

function handleCALL(EVM memory state) internal {

StackForCall memory stack;
stack.gas = state.stack.pop();
stack.target = address(state.stack.pop());
stack.value = state.stack.pop();
stack.inOffset = state.stack.pop();
stack.inSize = state.stack.pop();
stack.retOffset = state.stack.pop();
stack.retSize = state.stack.pop();

uint gasFee = computeGasForMemory(state, stack.retOffset + stack.retSize, stack.inOffset + stack.inSize);

if (gasFee > state.gas) {
state.gas = 0;
state.errno = ERROR_OUT_OF_GAS;
return;
}
state.gas -= gasFee;

bytes memory cd = state.mem.toBytes(stack.inOffset, stack.inSize);
Copy link
Member

Choose a reason for hiding this comment

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

maybe use cData or callDataBytes here, to keep it readable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

bytes4 funSig;
if (cd.length > 3) {
funSig = getSig(cd);
}

bool success;
bytes memory returnData;

if (funSig == 0x22334455) {
Copy link
Member

Choose a reason for hiding this comment

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

should be 0xa9059cbb for transfer(address,uint256)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed and moved to constants.

// transfer
address dest;
uint amount;

assembly {
dest := mload(add(cd,24))
Copy link
Member

Choose a reason for hiding this comment

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

indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

amount := mload(add(cd, 68))
}
EVMTokenBag.TransferParams memory params = EVMTokenBag.TransferParams({
color: stack.target,
from: state.caller,
to: dest,
value: amount
});
success = state.tokenBag.transfer(params);
Copy link
Member

Choose a reason for hiding this comment

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

could the creation of params be done inline, to save a stack element?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was actually not needed anymore. Removed in favour of normal function params.

returnData = abi.encodePacked(success);
} else {
state.errno = ERROR_INSTRUCTION_NOT_SUPPORTED;
return;
}

if (!success) {
state.stack.push(0);
Copy link
Member

Choose a reason for hiding this comment

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

indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

state.returnData = new bytes(0);
return;
}

state.stack.push(1);
state.mem.storeBytesAndPadWithZeroes(returnData, 0, stack.retOffset, stack.retSize);
state.returnData = returnData;
}

function handleCALLCODE(EVM memory state) internal {
Expand Down Expand Up @@ -1609,7 +1690,13 @@ contract EVMRuntime is EVMConstants {
}
state.gas -= retEvm.gas;

retEvm.data = state.mem.toBytes(inOffset, inSize);
bytes memory cd = state.mem.toBytes(inOffset, inSize);
Copy link
Member

Choose a reason for hiding this comment

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

indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

bytes4 funSig;
if (cd.length > 3) {
funSig = getSig(cd);
}

retEvm.data = cd;
retEvm.customDataPtr = state.customDataPtr;

// we only going to support precompiles
Expand All @@ -1623,15 +1710,40 @@ contract EVMRuntime is EVMConstants {
} else if (target == 4) {
handlePreC_IDENTITY(retEvm);
} else if (target == 5) {
handlePreC_MODEXP(retEvm);
handlePreC_MODEXP(retEvm);
} else if (target == 6) {
handlePreC_ECADD(retEvm);
} else if (target == 7) {
handlePreC_ECMUL(retEvm);
} else if (target == 8) {
handlePreC_ECPAIRING(retEvm);
}
} else {
} else if (funSig == 0x70a08231) {
// balanceOf
Copy link
Member

Choose a reason for hiding this comment

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

balanceOf(address) <- datatypes impact the sighash

Copy link
Contributor Author

@TheReturnOfJan TheReturnOfJan Jan 13, 2020

Choose a reason for hiding this comment

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

address addr;
// [32 length, 4 funSig, 20 address]
// swallow 4 for funSig and 20 for length
assembly {
addr := mload(add(cd,24))
}
uint value = state.tokenBag.balanceOf(address(target), addr);
bytes memory ret = abi.encodePacked(bytes32(value));

retEvm.returnData = ret;
} else if (funSig == 0x12341234) {
Copy link
Member

Choose a reason for hiding this comment

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

this should be 0x37ebbc03 for readData(uint256)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed and moved to constants.

// readData
uint tokenId;
// 32 length + 4 funcSig = 36
assembly {
tokenId := mload(add(cd,36))
}
bytes32 data = state.tokenBag.readData(address(target), tokenId);
bytes memory ret = abi.encodePacked(data);

// what happens here if we enter token intercepts instead of precompiles
retEvm.returnData = ret;
}
else {
retEvm.errno = ERROR_INSTRUCTION_NOT_SUPPORTED;
}

Expand Down
102 changes: 102 additions & 0 deletions contracts/EVMTokenBag.slb
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
pragma solidity ^0.5.2;
pragma experimental ABIEncoderV2;


library EVMTokenBag {

struct Output {
address owner;
uint valueOrId;
bytes32 data;
address color;
}

struct TokenBag {
Output[16] bag;
}

function balanceOf(
TokenBag memory self,
address color,
address owner
) internal pure returns (uint value) {
Output memory output;
for (uint i = 0; i < self.bag.length; i++) {
output = self.bag[i];
if (output.owner == owner && output.color == color) {
Copy link
Member

Choose a reason for hiding this comment

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

even if we assume that the tokenbag is defragmented before, it can happen that it gets fragmented through transfer() during execution. either we have to build the defragmentation into transfer(), or assume fragmentation here, and have the loop run till the end and sum everything up?

Copy link
Member

Choose a reason for hiding this comment

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

solved in call

value = output.valueOrId;
break;
}
}
}

function readData(
TokenBag memory self,
address color,
uint id
) internal pure returns (bytes32 data) {
Output memory output;
for (uint i = 0; i < self.bag.length; i++) {
output = self.bag[i];
if (output.valueOrId == id && output.color == color) {
data = output.data;
break;
}
}
}

struct TransferParams {
address color;
address from;
address to;
uint value;
}

function transfer(
TokenBag memory self,
TransferParams memory params
) internal pure returns (bool) {
Output memory source;
Output memory dest;
// find source
for (uint i = 0; i < self.bag.length; i++) {
if (self.bag[i].owner == params.from && self.bag[i].color == params.color) {
source = self.bag[i];
break;
}
}

// sender does not have enough tokens to send
if (source.valueOrId < params.value) return false;
Copy link
Member

Choose a reason for hiding this comment

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

would this work for data token transfers? if not, let's document the assumptions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

http://erc721.org/

ERC721 (and hence ERC1948) do not have a transfer method, only transferFrom. Hence the function signature will be different. Will deal with this problem down the road XD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we probably want to have both transfer and transferFrom share most of the implementation.


// find dest and/or empty
uint emptyId = 1337;
for (uint i = 0; i < self.bag.length; i++) {
if (self.bag[i].owner == params.to && self.bag[i].color == params.color) {
dest = self.bag[i];
break;
}
// check if empty slot
if (self.bag[i].owner == address(0) && emptyId == 1337) {
emptyId = i;
}
}

// if no dest and no empty slot, throw
if (dest.owner == address(0) && emptyId == 1337) return false;

// if no dest, but we found an empty slot, assign empty slot to reciver
if (dest.owner == address(0)) {
self.bag[emptyId].owner = params.to;
self.bag[emptyId].color = params.color;
dest = self.bag[emptyId];
}

// now do the state transition and return success
// OVERFLOWS???????
source.valueOrId -= params.value;
Copy link
Member

Choose a reason for hiding this comment

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

same question about data token, where value is actually ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above.

dest.valueOrId += params.value;
return true;
}

}
6 changes: 6 additions & 0 deletions contracts/EthereumRuntime.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma experimental ABIEncoderV2;
import { EVMCode } from "./EVMCode.slb";
import { EVMStack } from "./EVMStack.slb";
import { EVMMemory } from "./EVMMemory.slb";
import { EVMTokenBag } from "./EVMTokenBag.slb";
import { HydratedRuntime } from "./HydratedRuntime.sol";


Expand All @@ -20,6 +21,7 @@ contract EthereumRuntime is HydratedRuntime {
bytes32[] stack;
bytes32[] mem;
bytes returnData;
EVMTokenBag.TokenBag tokenBag;
}

struct EVMResult {
Expand All @@ -31,6 +33,7 @@ contract EthereumRuntime is HydratedRuntime {
bytes32[] stack;
uint pc;
bytes32 hashValue;
EVMTokenBag.TokenBag tokenBag;
}

// Init EVM with given stack and memory and execute from the given opcode
Expand All @@ -53,6 +56,8 @@ contract EthereumRuntime is HydratedRuntime {
evm.stack = EVMStack.fromArray(img.stack);
evm.mem = EVMMemory.fromArray(img.mem);

evm.tokenBag = img.tokenBag;
Copy link
Member

Choose a reason for hiding this comment

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

indentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


_run(evm, img.pc, img.stepCount);

bytes32 hashValue = stateHash(evm);
Expand All @@ -66,6 +71,7 @@ contract EthereumRuntime is HydratedRuntime {
resultState.stack = EVMStack.toArray(evm.stack);
resultState.pc = evm.pc;
resultState.hashValue = hashValue;
resultState.tokenBag = evm.tokenBag;

return resultState;
}
Expand Down
12 changes: 10 additions & 2 deletions test/contracts/ethereumRuntime.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ const fs = require('fs');
const assert = require('assert');

const { getCode, deployContract, deployCode } =
require('./../helpers/utils');
require('./../helpers/utils');
const { assertTokenBagEqual } = require('./../helpers/tokenBag.js');
const fixtures = require('./../fixtures/runtime');
const runtimeGasUsed = require('./../fixtures/runtimeGasUsed');
const Runtime = require('./../../utils/EthereumRuntimeAdapter');
Expand Down Expand Up @@ -42,6 +43,7 @@ describe('Runtime', function () {
stepCount: stepCount,
}
)).pc;

assert.equal(await executeStep(1), 2, 'should be at 2 JUMP');
assert.equal(await executeStep(2), 8, 'should be at 8 JUMPDEST');
assert.equal(await executeStep(3), 9, 'should be at 9 PUSH1');
Expand Down Expand Up @@ -85,6 +87,7 @@ describe('Runtime', function () {
const stack = fixture.stack || [];
const mem = fixture.memory || [];
const data = fixture.data || '0x';
const tokenBag = fixture.tokenBag || undefined;
const gasLimit = fixture.gasLimit || BLOCK_GAS_LIMIT;
const gasRemaining = typeof fixture.gasRemaining !== 'undefined' ? fixture.gasRemaining : gasLimit;
const codeContract = await deployCode(code);
Expand All @@ -96,8 +99,10 @@ describe('Runtime', function () {
gasRemaining,
stack,
mem,
tokenBag,
};
const res = await rt.execute(args);

const gasUsed = (await (await rt.execute(args, true)).wait()).gasUsed.toNumber();

totalGasUsed += gasUsed;
Expand All @@ -108,7 +113,7 @@ describe('Runtime', function () {

if (gasUsedBaseline !== undefined) {
// The max increase in gas usage
const maxAllowedDiff = 5000;
const maxAllowedDiff = 500000;
Copy link
Member

Choose a reason for hiding this comment

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

maybe create an issue to kick this out ASAP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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


// Skip gas accounting if we do coverage.
// Ther other hack is for ganache. It has wrong gas accounting with some precompiles 🤦
Expand All @@ -134,6 +139,9 @@ describe('Runtime', function () {
if (fixture.result.memory) {
assert.deepEqual(res.mem, fixture.result.memory, 'memory');
}
if (fixture.result.tokenBag) {
assertTokenBagEqual(fixture.result.tokenBag, res.tokenBag);
}
if (fixture.result.pc !== undefined) {
assert.equal(res.pc.toNumber(), fixture.result.pc, 'pc');
}
Expand Down
Loading