Merge pull request #826 from 0xProject/refactor/contracts/todos

Address TODOs and small fixes
This commit is contained in:
Amir Bandeali 2018-07-06 13:41:06 -07:00 committed by GitHub
commit e929fb4337
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
30 changed files with 49 additions and 65 deletions

View File

@ -24,7 +24,7 @@
"compile": "sol-compiler --contracts-dir src", "compile": "sol-compiler --contracts-dir src",
"clean": "shx rm -rf lib generated_contract_wrappers", "clean": "shx rm -rf lib generated_contract_wrappers",
"generate_contract_wrappers": "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers", "generate_contract_wrappers": "abi-gen --abis ${npm_package_config_abis} --template ../contract_templates/contract.handlebars --partials '../contract_templates/partials/**/*.handlebars' --output generated_contract_wrappers --backend ethers",
"lint": "tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/*", "lint": "tslint --project . --exclude **/src/generated_contract_wrappers/**/* --exclude **/lib/**/* && yarn lint-contracts",
"coverage:report:text": "istanbul report text", "coverage:report:text": "istanbul report text",
"coverage:report:html": "istanbul report html && open coverage/index.html", "coverage:report:html": "istanbul report html && open coverage/index.html",
"profiler:report:html": "istanbul report html && open coverage/index.html", "profiler:report:html": "istanbul report html && open coverage/index.html",

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
contract MixinERC20 { contract MixinERC20 {

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import { WETH9 as EtherToken } from "../tokens/WETH9/WETH9.sol"; import { WETH9 as EtherToken } from "../tokens/WETH9/WETH9.sol";
import "../protocol/Exchange/libs/LibMath.sol"; import "../protocol/Exchange/libs/LibMath.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol"; import "../../utils/LibBytes/LibBytes.sol";
import "./MixinAuthorizable.sol"; import "./MixinAuthorizable.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol"; import "../../utils/LibBytes/LibBytes.sol";
import "./MixinAuthorizable.sol"; import "./MixinAuthorizable.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../utils/Ownable/Ownable.sol"; import "../../utils/Ownable/Ownable.sol";
import "./mixins/MAuthorizable.sol"; import "./mixins/MAuthorizable.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "./IAuthorizable.sol"; import "./IAuthorizable.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../../utils/Ownable/IOwnable.sol"; import "../../../utils/Ownable/IOwnable.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../interfaces/IAuthorizable.sol"; import "../interfaces/IAuthorizable.sol";

View File

@ -374,21 +374,19 @@ contract MixinExchangeCore is
returns (FillResults memory fillResults) returns (FillResults memory fillResults)
{ {
// Compute proportional transfer amounts // Compute proportional transfer amounts
// TODO: All three are multiplied by the same fraction. This can
// potentially be optimized.
fillResults.takerAssetFilledAmount = takerAssetFilledAmount; fillResults.takerAssetFilledAmount = takerAssetFilledAmount;
fillResults.makerAssetFilledAmount = getPartialAmount( fillResults.makerAssetFilledAmount = getPartialAmount(
fillResults.takerAssetFilledAmount, takerAssetFilledAmount,
order.takerAssetAmount, order.takerAssetAmount,
order.makerAssetAmount order.makerAssetAmount
); );
fillResults.makerFeePaid = getPartialAmount( fillResults.makerFeePaid = getPartialAmount(
fillResults.takerAssetFilledAmount, takerAssetFilledAmount,
order.takerAssetAmount, order.takerAssetAmount,
order.makerFee order.makerFee
); );
fillResults.takerFeePaid = getPartialAmount( fillResults.takerFeePaid = getPartialAmount(
fillResults.takerAssetFilledAmount, takerAssetFilledAmount,
order.takerAssetAmount, order.takerAssetAmount,
order.takerFee order.takerFee
); );

View File

@ -41,7 +41,6 @@ contract MixinMatchOrders is
/// @param leftSignature Proof that order was created by the left maker. /// @param leftSignature Proof that order was created by the left maker.
/// @param rightSignature Proof that order was created by the right maker. /// @param rightSignature Proof that order was created by the right maker.
/// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders. /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
/// TODO: Make this function external once supported by Solidity (See Solidity Issues #3199, #1603)
function matchOrders( function matchOrders(
LibOrder.Order memory leftOrder, LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder, LibOrder.Order memory rightOrder,
@ -184,7 +183,6 @@ contract MixinMatchOrders is
leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining; leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining;
// The right order receives an amount proportional to how much was spent. // The right order receives an amount proportional to how much was spent.
// TODO: Can we ensure rounding error is in the correct direction?
rightTakerAssetFilledAmount = getPartialAmount( rightTakerAssetFilledAmount = getPartialAmount(
rightOrder.takerAssetAmount, rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount, rightOrder.makerAssetAmount,
@ -195,7 +193,6 @@ contract MixinMatchOrders is
rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining; rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining;
// The left order receives an amount proportional to how much was spent. // The left order receives an amount proportional to how much was spent.
// TODO: Can we ensure rounding error is in the correct direction?
leftTakerAssetFilledAmount = getPartialAmount( leftTakerAssetFilledAmount = getPartialAmount(
rightOrder.makerAssetAmount, rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount, rightOrder.takerAssetAmount,

View File

@ -91,7 +91,6 @@ contract MixinSignatureValidator is
view view
returns (bool isValid) returns (bool isValid)
{ {
// TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.)
require( require(
signature.length > 0, signature.length > 0,
"LENGTH_GREATER_THAN_0_REQUIRED" "LENGTH_GREATER_THAN_0_REQUIRED"

View File

@ -99,9 +99,10 @@ contract MixinTransactions is
"FAILED_EXECUTION" "FAILED_EXECUTION"
); );
// Reset current transaction signer // Reset current transaction signer if it was previously updated
// TODO: Check if gas is paid when currentContextAddress is already 0. if (signerAddress != msg.sender) {
currentContextAddress = address(0); currentContextAddress = address(0);
}
} }
/// @dev Calculates EIP712 hash of the Transaction. /// @dev Calculates EIP712 hash of the Transaction.
@ -120,13 +121,15 @@ contract MixinTransactions is
{ {
bytes32 schemaHash = EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH; bytes32 schemaHash = EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH;
bytes32 dataHash = keccak256(data); bytes32 dataHash = keccak256(data);
// Assembly for more efficiently computing: // Assembly for more efficiently computing:
// keccak256(abi.encode( // keccak256(abi.encode(
// EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH, // EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH,
// salt, // salt,
// signerAddress, // signerAddress,
// keccak256(data) // keccak256(data)
// )); // ));
assembly { assembly {
let memPtr := mload(64) let memPtr := mload(64)
mstore(memPtr, schemaHash) mstore(memPtr, schemaHash)

View File

@ -33,7 +33,6 @@ contract IMatchOrders {
/// @param leftSignature Proof that order was created by the left maker. /// @param leftSignature Proof that order was created by the left maker.
/// @param rightSignature Proof that order was created by the right maker. /// @param rightSignature Proof that order was created by the right maker.
/// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders. /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
/// TODO: Make this function external once supported by Solidity (See Solidity Issues #3199, #1603)
function matchOrders( function matchOrders(
LibOrder.Order memory leftOrder, LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder, LibOrder.Order memory rightOrder,

View File

@ -24,6 +24,7 @@ import "../libs/LibFillResults.sol";
contract IWrapperFunctions { contract IWrapperFunctions {
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
/// @param order LibOrder.Order struct containing order specifications. /// @param order LibOrder.Order struct containing order specifications.
/// @param takerAssetFillAmount Desired amount of takerAsset to sell. /// @param takerAssetFillAmount Desired amount of takerAsset to sell.

View File

@ -101,21 +101,23 @@ contract LibOrder is
bytes32 schemaHash = EIP712_ORDER_SCHEMA_HASH; bytes32 schemaHash = EIP712_ORDER_SCHEMA_HASH;
bytes32 makerAssetDataHash = keccak256(order.makerAssetData); bytes32 makerAssetDataHash = keccak256(order.makerAssetData);
bytes32 takerAssetDataHash = keccak256(order.takerAssetData); bytes32 takerAssetDataHash = keccak256(order.takerAssetData);
// Assembly for more efficiently computing: // Assembly for more efficiently computing:
// keccak256(abi.encode( // keccak256(abi.encode(
// order.makerAddress, // order.makerAddress,
// order.takerAddress, // order.takerAddress,
// order.feeRecipientAddress, // order.feeRecipientAddress,
// order.senderAddress, // order.senderAddress,
// order.makerAssetAmount, // order.makerAssetAmount,
// order.takerAssetAmount, // order.takerAssetAmount,
// order.makerFee, // order.makerFee,
// order.takerFee, // order.takerFee,
// order.expirationTimeSeconds, // order.expirationTimeSeconds,
// order.salt, // order.salt,
// keccak256(order.makerAssetData), // keccak256(order.makerAssetData),
// keccak256(order.takerAssetData) // keccak256(order.takerAssetData)
// )); // ));
assembly { assembly {
// Backup // Backup
// solhint-disable-next-line space-after-comma // solhint-disable-next-line space-after-comma

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../interfaces/IAssetProxyDispatcher.sol"; import "../interfaces/IAssetProxyDispatcher.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../Mintable/Mintable.sol"; import "../Mintable/Mintable.sol";
import "../../utils/Ownable/Ownable.sol"; import "../../utils/Ownable/Ownable.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../tokens/ERC721Token/ERC721Token.sol"; import "../../tokens/ERC721Token/ERC721Token.sol";
import "../../utils/Ownable/Ownable.sol"; import "../../utils/Ownable/Ownable.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../tokens/UnlimitedAllowanceToken/UnlimitedAllowanceToken.sol"; import "../../tokens/UnlimitedAllowanceToken/UnlimitedAllowanceToken.sol";
import "../../utils/SafeMath/SafeMath.sol"; import "../../utils/SafeMath/SafeMath.sol";
@ -27,7 +26,10 @@ import "../../utils/SafeMath/SafeMath.sol";
* Mintable * Mintable
* Base contract that creates a mintable UnlimitedAllowanceToken * Base contract that creates a mintable UnlimitedAllowanceToken
*/ */
contract Mintable is UnlimitedAllowanceToken, SafeMath { contract Mintable is
UnlimitedAllowanceToken,
SafeMath
{
function mint(uint256 _value) function mint(uint256 _value)
public public
{ {

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol"; import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol";
@ -27,7 +26,8 @@ contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher {
bytes memory assetData, bytes memory assetData,
address from, address from,
address to, address to,
uint256 amount) uint256 amount
)
public public
{ {
dispatchTransferFrom(assetData, from, to, amount); dispatchTransferFrom(assetData, from, to, amount);

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol"; import "../../utils/LibBytes/LibBytes.sol";

View File

@ -32,7 +32,8 @@ contract TestLibs is
function publicGetPartialAmount( function publicGetPartialAmount(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target) uint256 target
)
public public
pure pure
returns (uint256 partialAmount) returns (uint256 partialAmount)
@ -48,7 +49,8 @@ contract TestLibs is
function publicIsRoundingError( function publicIsRoundingError(
uint256 numerator, uint256 numerator,
uint256 denominator, uint256 denominator,
uint256 target) uint256 target
)
public public
pure pure
returns (bool isError) returns (bool isError)

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../../protocol/Exchange/MixinSignatureValidator.sol"; import "../../protocol/Exchange/MixinSignatureValidator.sol";
import "../../protocol/Exchange/MixinTransactions.sol"; import "../../protocol/Exchange/MixinTransactions.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "./IERC20Token.sol"; import "./IERC20Token.sol";

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
contract IERC20Token { contract IERC20Token {

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
import "../ERC20Token/ERC20Token.sol"; import "../ERC20Token/ERC20Token.sol";

View File

@ -1,5 +1,4 @@
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
/* /*
* Ownable * Ownable

View File

@ -1,5 +1,4 @@
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
/* /*
* Ownable * Ownable

View File

@ -1,28 +1,27 @@
pragma solidity 0.4.24; pragma solidity 0.4.24;
pragma experimental ABIEncoderV2;
contract SafeMath { contract SafeMath {
function safeMul(uint a, uint b) function safeMul(uint256 a, uint256 b)
internal internal
pure pure
returns (uint256) returns (uint256)
{ {
uint c = a * b; uint256 c = a * b;
assert(a == 0 || c / a == b); assert(a == 0 || c / a == b);
return c; return c;
} }
function safeDiv(uint a, uint b) function safeDiv(uint256 a, uint256 b)
internal internal
pure pure
returns (uint256) returns (uint256)
{ {
uint c = a / b; uint256 c = a / b;
return c; return c;
} }
function safeSub(uint a, uint b) function safeSub(uint256 a, uint256 b)
internal internal
pure pure
returns (uint256) returns (uint256)
@ -31,12 +30,12 @@ contract SafeMath {
return a - b; return a - b;
} }
function safeAdd(uint a, uint b) function safeAdd(uint256 a, uint256 b)
internal internal
pure pure
returns (uint256) returns (uint256)
{ {
uint c = a + b; uint256 c = a + b;
assert(c >= a); assert(c >= a);
return c; return c;
} }