Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Implement StaticCallProxy #1863

Merged
merged 16 commits into from
Jun 21, 2019
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
17 changes: 8 additions & 9 deletions contracts/asset-proxy/CHANGELOG.json
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
[
{
"version": "2.3.0",
"changes": [
{
"note": "Updated ERC1155 Asset Proxy. Less optimization. More explicit handling of edge cases.",
"pr": 1852
}
]
},
{
"version": "2.2.0",
"changes": [
{
"note": "Add `LibAssetProxyIds` contract",
"pr": 1835
},
{
"note": "Updated ERC1155 Asset Proxy. Less optimization. More explicit handling of edge cases.",
"pr": 1852
},
{
"note": "Implement StaticCallProxy",
"pr": 1863
}
]
},
Expand Down
4 changes: 3 additions & 1 deletion contracts/asset-proxy/compiler.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@
"src/ERC721Proxy.sol",
"src/MixinAuthorizable.sol",
"src/MultiAssetProxy.sol",
"src/StaticCallProxy.sol",
"src/interfaces/IAssetData.sol",
"src/interfaces/IAssetProxy.sol",
"src/interfaces/IAuthorizable.sol"
"src/interfaces/IAuthorizable.sol",
"test/TestStaticCallTarget.sol"
]
}
6 changes: 3 additions & 3 deletions contracts/asset-proxy/contracts/src/ERC1155Proxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

*/

pragma solidity ^0.5.5;
pragma solidity ^0.5.9;

import "./MixinAuthorizable.sol";

Expand Down Expand Up @@ -179,10 +179,10 @@ contract ERC1155Proxy is
// +32 for length field
let assetDataEnd := add(assetDataOffset, add(assetDataLength, 32))
if gt(assetDataEnd, calldatasize()) {
// Revert with `Error("INVALID_ASSET_DATA")`
// Revert with `Error("INVALID_ASSET_DATA_END")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x00000012494e56414c49445f41535345545f4441544100000000000000000000)
mstore(64, 0x00000016494e56414c49445f41535345545f444154415f454e44000000000000)
mstore(96, 0)
revert(0, 100)
}
Expand Down
207 changes: 207 additions & 0 deletions contracts/asset-proxy/contracts/src/StaticCallProxy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
/*

Copyright 2018 ZeroEx Intl.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

*/

pragma solidity ^0.5.9;


contract StaticCallProxy {

// Id of this proxy.
bytes4 constant internal PROXY_ID = bytes4(keccak256("StaticCall(address,bytes,bytes32)"));

// solhint-disable-next-line payable-fallback
function ()
external
{
assembly {
// The first 4 bytes of calldata holds the function selector
let selector := and(calldataload(0), 0xffffffff00000000000000000000000000000000000000000000000000000000)

// `transferFrom` will be called with the following parameters:
// assetData Encoded byte array.
// from Address to transfer asset from.
// to Address to transfer asset to.
// amount Amount of asset to transfer.
// bytes4(keccak256("transferFrom(bytes,address,address,uint256)")) = 0xa85e59e4
if eq(selector, 0xa85e59e400000000000000000000000000000000000000000000000000000000) {

// `transferFrom`.
// The function is marked `external`, so no abi decoding is done for
// us. Instead, we expect the `calldata` memory to contain the
// following:
//
// | Area | Offset | Length | Contents |
// |----------|--------|---------|-------------------------------------|
// | Header | 0 | 4 | function selector |
// | Params | | 4 * 32 | function parameters: |
// | | 4 | | 1. offset to assetData (*) |
// | | 36 | | 2. from |
// | | 68 | | 3. to |
// | | 100 | | 4. amount |
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're moving towards localizing validation within the proxies, we may want to assert that amount is some fixed, standardized value -- like 0. Ex, in the MultiAssetProxy scaling this value should have no effect.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is that the Exchange contract doesn't allow 0 transfers. Any non-zero number would be scaled with the MAP.

// | Data | | | assetData: |
// | | 132 | 32 | assetData Length |
// | | 164 | ** | assetData Contents |
//
// (*): offset is computed from start of function parameters, so offset
// by an additional 4 bytes in the calldata.
//
// (**): see table below to compute length of assetData Contents
// (***): Note that the `from`, `to`, and `amount` params in calldata are ignored in this function.
//
// WARNING: The ABIv2 specification allows additional padding between
// the Params and Data section. This will result in a larger
// offset to assetData.

// Load offset to `assetData`
let assetDataOffset := add(calldataload(4), 4)

// Validate length of `assetData`
let assetDataLen := calldataload(assetDataOffset)
if or(lt(assetDataLen, 100), mod(sub(assetDataLen, 4), 32)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if or(lt(assetDataLen, 100), mod(sub(assetDataLen, 4), 32)) {
if lt(assetDataLen, 132) {
  • staticCallData Length should always be part of the asset data regardless, no?
  • Do we need to enforce that the entire asset data be word-aligned? It's conceivable to me that an abi-encoder (though probably not ours) would not word-align parameters such as bytes types.

Copy link
Member Author

Choose a reason for hiding this comment

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

bytes, of length k (which is assumed to be of type uint256):

enc(X) = enc(k) pad_right(X), i.e. the number of bytes is encoded as a uint256 followed by the actual value of X as a byte sequence, followed by the minimum number of zero-bytes such that len(enc(X)) is a multiple of 32.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. I guess this also explains why we word align revert strings.

// Revert with `Error("INVALID_ASSET_DATA_LENGTH")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x00000019494e56414c49445f41535345545f444154415f4c454e475448000000)
mstore(96, 0)
revert(0, 100)
}

// Ensure that `assetData` ends inside of calldata
let assetDataEnd := add(assetDataOffset, add(assetDataLen, 32))
if gt(assetDataEnd, calldatasize()) {
// Revert with `Error("INVALID_ASSET_DATA_END")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x00000016494e56414c49445f41535345545f444154415f454e44000000000000)
mstore(96, 0)
revert(0, 100)
}

// Asset data is encoded as follows:
// | Area | Offset | Length | Contents |
// |----------|-------------|---------|--------------------------------------|
// | Header | 0 | 4 | assetProxyId |
// | Params | | 4 * 32 | function parameters: |
// | | 4 | | 1. address of callTarget |
// | | 36 | | 2. offset to staticCallData (*) |
// | | 68 | | 3. expected 32 byte hash of output |
// | Data | | | staticCallData: |
// | | 100 | 32 | 1. staticCallData Length |
// | | 132 | a | 2. staticCallData Contents |

// In order to find the offset to `staticCallData`, we must add:
// assetDataOffset
// + 32 (assetData len)
// + 4 (proxyId)
// + 32 (callTarget)
let paramsInAssetDataOffset := add(assetDataOffset, 36)
let staticCallDataOffset := add(paramsInAssetDataOffset, calldataload(add(assetDataOffset, 68)))

// Load length of `staticCallData`
let staticCallDataLen := calldataload(staticCallDataOffset)

// Ensure `staticCallData` does not begin to outside of `assetData`
let staticCallDataBegin := add(staticCallDataOffset, 32)
let staticCallDataEnd := add(staticCallDataBegin, staticCallDataLen)
if gt(staticCallDataEnd, assetDataEnd) {
// Revert with `Error("INVALID_STATIC_CALL_DATA_OFFSET")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x0000001f494e56414c49445f5354415449435f43414c4c5f444154415f4f4646)
mstore(96, 0x5345540000000000000000000000000000000000000000000000000000000000)
revert(0, 100)
}

// Copy `staticCallData` into memory
calldatacopy(
0, // memory can be safely overwritten from beginning
staticCallDataBegin, // start of `staticCallData`
staticCallDataLen // copy the entire `staticCallData`
)

// In order to find the offset to `callTarget`, we must add:
// assetDataOffset
// + 32 (assetData len)
// + 4 (proxyId)
let callTarget := and(
calldataload(add(assetDataOffset, 36)),
0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff
)

// Perform `callTarget.staticcall(staticCallData)`
let success := staticcall(
gas, // forward all gas
callTarget, // call address `callTarget`
0, // pointer to start of input
staticCallDataLen, // length of input
0, // start of memory can be safely overwritten
0 // don't copy output to memory
)

// Copy entire output to start of memory
let outputLen := returndatasize()
returndatacopy(
0, // copy to memory at 0
0, // copy from return data at 0
outputLen // copy all return data
)

// Revert with reason given by `callTarget` if staticcall is unsuccessful
if iszero(success) {
revert(0, outputLen)
}

// Calculate hash of output
let callResultHash := keccak256(0, outputLen)

// In order to find the offset to `expectedCallResultHash`, we must add:
// assetDataOffset
// + 32 (assetData len)
// + 4 (proxyId)
// + 32 (callTarget)
// + 32 (staticCallDataOffset)
let expectedResultHash := calldataload(add(assetDataOffset, 100))

if sub(callResultHash, expectedResultHash) {
// Revert with `Error("UNEXPECTED_STATIC_CALL_RESULT")`
mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
mstore(64, 0x0000001d554e45585045435445445f5354415449435f43414c4c5f524553554c)
mstore(96, 0x5400000000000000000000000000000000000000000000000000000000000000)
revert(0, 100)
}

// Return if output matched expected output
return(0, 0)
}

// Revert if undefined function is called
revert(0, 0)
}
}

/// @dev Gets the proxy id associated with the proxy address.
/// @return Proxy id.
function getProxyId()
external
pure
returns (bytes4)
{
return PROXY_ID;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,11 @@ interface IAssetData {
bytes[] calldata nestedAssetData
)
external;


function StaticCall(
address callTarget,
bytes calldata staticCallData,
bytes32 callResultHash
)
external;
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,7 @@ contract LibAssetProxyIds {

// MultiAsset(uint256[],bytes[])
bytes4 constant public MULTI_ASSET_PROXY_ID = 0x94cfcdd7;

// StaticCall(address,bytes,bytes32)
bytes4 constant public STATIC_CALL_PROXY_ID = 0xc339d10a;
}
82 changes: 82 additions & 0 deletions contracts/asset-proxy/contracts/test/TestStaticCallTarget.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*

Copyright 2018 ZeroEx Intl.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.

*/

pragma solidity ^0.5.9;

import "@0x/contracts-utils/contracts/src/LibBytes.sol";


contract TestStaticCallTarget {

using LibBytes for bytes;

uint256 internal _state;

function updateState()
external
{
_state++;
}

function assertEvenNumber(uint256 target)
external
pure
{
require(
target % 2 == 0,
"TARGET_NOT_EVEN"
);
}

function isOddNumber(uint256 target)
external
pure
returns (bool isOdd)
{
isOdd = target % 2 == 1;
return isOdd;
}

function noInputFunction()
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be good to do a sanity check to ensure that calldata() == <selector> without any extra data.

external
pure
{
assert(msg.data.length == 4 && msg.data.readBytes4(0) == bytes4(keccak256("noInputFunction()")));
}

function dynamicInputFunction(bytes calldata a)
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be good to do some sanity check on the input data, to ensure it was forwarded correctly.

external
pure
{
bytes memory abiEncodedData = abi.encodeWithSignature("dynamicInputFunction(bytes)", a);
assert(msg.data.equals(abiEncodedData));
}

function returnComplexType(uint256 a, uint256 b)
external
view
returns (bytes memory result)
{
result = abi.encodePacked(
address(this),
a,
b
);
return result;
}
}
2 changes: 1 addition & 1 deletion contracts/asset-proxy/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol"
},
"config": {
"abis": "./generated-artifacts/@(ERC1155Proxy|ERC20Proxy|ERC721Proxy|IAssetData|IAssetProxy|IAuthorizable|MixinAuthorizable|MultiAssetProxy).json",
"abis": "./generated-artifacts/@(ERC1155Proxy|ERC20Proxy|ERC721Proxy|IAssetData|IAssetProxy|IAuthorizable|MixinAuthorizable|MultiAssetProxy|StaticCallProxy|TestStaticCallTarget).json",
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
},
"repository": {
Expand Down
Loading