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

Refactor executeCall to use executeUnsignedEOACall #65

Merged
merged 18 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
62 changes: 23 additions & 39 deletions packages/ovm/src/contracts/ExecutionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ contract ExecutionManager is FullStateManager {
// bitwise right shift 28 * 8 bits so the 4 method ID bytes are in the right-most bytes
bytes32 constant ovmCallMethodId = keccak256("ovmCALL()") >> 224;
bytes32 constant ovmCreateMethodId = keccak256("ovmCREATE()") >> 224;
bytes32 constant executeCallMethodId = keccak256("executeCall()") >> 224;

// Precompile addresses
address constant l2ToL1MessagePasserOvmAddress = 0x4200000000000000000000000000000000000000;
Expand Down Expand Up @@ -106,7 +105,7 @@ contract ExecutionManager is FullStateManager {
}

/**
* @notice Execute a call which will return the result of the call instead of the updated storage.
* @notice Execute a transaction which will return the result of the call instead of the updated storage.
* Note: This should only be used with a Web3 `call` operation, otherwise you may accidentally save changes to the state.
* Note: This is a raw function, so there are no listed (ABI-encoded) inputs / outputs.
* Below format of the bytes expected as input and written as output:
Expand All @@ -118,52 +117,37 @@ contract ExecutionManager is FullStateManager {
* [callBytes (bytes (variable length))]
* returndata: [variable-length bytes returned from call]
*/
function executeCall() external {
function executeTransactionRaw() external {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would consider massaging the @notice above here for clarity as it currently says "Execute a call" and we are renaming the function from executeCall

uint _timestamp;
uint _queueOrigin;
uint callSize;
uint _callSize;
bytes memory callBytes;
bytes32 methodId = ovmCallMethodId;
address _ovmEntrypoint;
assembly {
// Revert if we don't have methodId, timestamp, queueOrigin, and ovmEntrypointAddress.
if lt(calldatasize, 100) {
revert(0,0)
}

// populate timestamp and queue origin from calldata
_timestamp := calldataload(4)
_timestamp := calldataload(0x04)
// skip method ID (bytes4) and timestamp (bytes32)
_queueOrigin := calldataload(0x24)

callBytes := mload(0x40)
// set callsize: total param size minus 2 uints (methodId bytes are repurposed)
callSize := sub(calldatasize, 0x40)
mstore(0x40, add(callBytes, callSize))

// leave room for method ID, skip ahead in calldata methodID(4), timestamp(32), queueOrigin(32)
calldatacopy(add(callBytes, 4), 0x44, sub(callSize, 4))

mstore8(callBytes, shr(24, methodId))
mstore8(add(callBytes, 1), shr(16, methodId))
mstore8(add(callBytes, 2), shr(8, methodId))
mstore8(add(callBytes, 3), methodId)
}

// Initialize our context
initializeContext(_timestamp, _queueOrigin, ZERO_ADDRESS, ZERO_ADDRESS);

address addr = address(this);
assembly {
let success := call(gas, addr, 0, callBytes, callSize, 0, 0)
let result := mload(0x40)
returndatacopy(result, 0, returndatasize)

if eq(success, 0) {
revert(result, returndatasize)
}
_callSize := sub(calldatasize, 0x40)
mstore(0x40, add(callBytes, _callSize))

return(result, returndatasize)
_ovmEntrypoint := calldataload(0x44)
calldatacopy(add(callBytes, 0x20), 0x64, sub(_callSize, 0x04))
mstore(callBytes, sub(_callSize, 0x20))
}

return executeTransaction(
_timestamp,
_queueOrigin,
_ovmEntrypoint,
callBytes,
ZERO_ADDRESS,
ZERO_ADDRESS,
true
);
}

/********************
Expand Down Expand Up @@ -201,11 +185,11 @@ contract ExecutionManager is FullStateManager {
require(_nonce == getOvmContractNonce(eoaAddress), "Incorrect nonce!");
emit CallingWithEOA(eoaAddress);
// Make the EOA call for the account
executeUnsignedEOACall(_timestamp, _queueOrigin, _ovmEntrypoint, _callBytes, eoaAddress, ZERO_ADDRESS, false);
executeTransaction(_timestamp, _queueOrigin, _ovmEntrypoint, _callBytes, eoaAddress, ZERO_ADDRESS, false);
}

/**
* @notice Execute an unsigned EOA call. Note that unsigned EOA calls are unauthenticated.
* @notice Execute an unsigned EOA transaction. Note that unsigned EOA calls are unauthenticated.
* This means that they should not be allowed for normal execution.
* @param _timestamp The timestamp which should be used for this call's context.
* @param _queueOrigin The parent-chain queue from which this call originated.
Expand All @@ -214,7 +198,7 @@ contract ExecutionManager is FullStateManager {
* @param _fromAddress The address which this call should originate from--the msg.sender.
* @param _allowRevert Flag which controls whether or not to revert in the case of failure.
*/
function executeUnsignedEOACall(
function executeTransaction(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above WRT @notice

uint _timestamp,
uint _queueOrigin,
address _ovmEntrypoint,
Expand Down
Loading