From 1dae1d244ca8cc2cd0734f8c21e3574d80670943 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 14 Aug 2019 13:38:12 -0700 Subject: [PATCH] Rename hashing functions in LibOrder and LibZeroExTransaction --- .../exchange-libs/contracts/src/LibOrder.sol | 26 +++++++++---------- .../contracts/src/LibZeroExTransaction.sol | 18 ++++++------- .../contracts/test/TestLibOrder.sol | 8 +++--- .../test/TestLibZeroExTransaction.sol | 8 +++--- contracts/exchange-libs/package.json | 2 +- contracts/exchange-libs/src/artifacts.ts | 2 -- contracts/exchange-libs/src/wrappers.ts | 1 - contracts/exchange-libs/test/lib_order.ts | 6 ++--- .../test/lib_zero_ex_transaction.ts | 6 ++--- contracts/exchange-libs/tsconfig.json | 1 - .../contracts/src/MixinExchangeCore.sol | 2 +- .../contracts/src/MixinSignatureValidator.sol | 4 +-- .../contracts/src/MixinTransactions.sol | 2 +- .../src/interfaces/IExchangeCore.sol | 6 ++--- .../contracts/test/TestExchangeInternals.sol | 2 +- .../contracts/test/TestValidatorWallet.sol | 4 +-- .../contracts/test/TestWrapperFunctions.sol | 4 +-- 17 files changed, 49 insertions(+), 53 deletions(-) diff --git a/contracts/exchange-libs/contracts/src/LibOrder.sol b/contracts/exchange-libs/contracts/src/LibOrder.sol index 423e934d73..250aaac5a4 100644 --- a/contracts/exchange-libs/contracts/src/LibOrder.sol +++ b/contracts/exchange-libs/contracts/src/LibOrder.sol @@ -48,7 +48,7 @@ library LibOrder { 0xf80322eb8376aafb64eadf8f0d7623f22130fd9491a221e902b713cb984a7534; // A valid order remains fillable until it is expired, fully filled, or cancelled. - // An order's state is unaffected by external factors, like account balances. + // An order's status is unaffected by external factors, like account balances. enum OrderStatus { INVALID, // Default value INVALID_MAKER_ASSET_AMOUNT, // Order does not have a valid maker asset amount @@ -67,42 +67,42 @@ library LibOrder { address senderAddress; // Address that is allowed to call Exchange contract methods that affect this order. If set to 0, any address is allowed to call these methods. uint256 makerAssetAmount; // Amount of makerAsset being offered by maker. Must be greater than 0. uint256 takerAssetAmount; // Amount of takerAsset being bid on by maker. Must be greater than 0. - uint256 makerFee; // Amount of ZRX paid to feeRecipient by maker when order is filled. If set to 0, no transfer of ZRX from maker to feeRecipient will be attempted. - uint256 takerFee; // Amount of ZRX paid to feeRecipient by taker when order is filled. If set to 0, no transfer of ZRX from taker to feeRecipient will be attempted. + uint256 makerFee; // Fee paid to feeRecipient by maker when order is filled. + uint256 takerFee; // Fee paid to feeRecipient by taker when order is filled. uint256 expirationTimeSeconds; // Timestamp in seconds at which order expires. uint256 salt; // Arbitrary number to facilitate uniqueness of the order's hash. bytes makerAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring makerAsset. The leading bytes4 references the id of the asset proxy. bytes takerAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring takerAsset. The leading bytes4 references the id of the asset proxy. - bytes makerFeeAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring makerAsset fees. The leading bytes4 references the id of the asset proxy. - bytes takerFeeAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring takerAsset fees. The leading bytes4 references the id of the asset proxy. + bytes makerFeeAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring makerFeeAsset. The leading bytes4 references the id of the asset proxy. + bytes takerFeeAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring takerFeeAsset. The leading bytes4 references the id of the asset proxy. } // solhint-enable max-line-length struct OrderInfo { uint8 orderStatus; // Status that describes order's validity and fillability. - bytes32 orderHash; // EIP712 hash of the order (see LibOrder.getOrderHash). + bytes32 orderHash; // EIP712 typed data hash of the order (see LibOrder.getTypedDataHash). uint256 orderTakerAssetFilledAmount; // Amount of order that has already been filled. } - /// @dev Calculates Keccak-256 hash of the order. + /// @dev Calculates the EIP712 typed data hash of an order with a given domain separator. /// @param order The order structure. - /// @return Keccak-256 EIP712 hash of the order. - function getOrderHash(Order memory order, bytes32 eip712ExchangeDomainHash) + /// @return EIP712 typed data hash of the order. + function getTypedDataHash(Order memory order, bytes32 eip712ExchangeDomainHash) internal pure returns (bytes32 orderHash) { orderHash = LibEIP712.hashEIP712Message( eip712ExchangeDomainHash, - order.hashOrder() + order.getStructHash() ); return orderHash; } - /// @dev Calculates EIP712 hash of the order. + /// @dev Calculates EIP712 hash of the order struct. /// @param order The order structure. - /// @return EIP712 hash of the order. - function hashOrder(Order memory order) + /// @return EIP712 hash of the order struct. + function getStructHash(Order memory order) internal pure returns (bytes32 result) diff --git a/contracts/exchange-libs/contracts/src/LibZeroExTransaction.sol b/contracts/exchange-libs/contracts/src/LibZeroExTransaction.sol index 2b7a982ccd..20cf3ec74d 100644 --- a/contracts/exchange-libs/contracts/src/LibZeroExTransaction.sol +++ b/contracts/exchange-libs/contracts/src/LibZeroExTransaction.sol @@ -44,10 +44,10 @@ library LibZeroExTransaction { bytes data; // AbiV2 encoded calldata. } - /// @dev Calculates the EIP712 hash of a 0x transaction using the domain separator of the Exchange contract. - /// @param transaction 0x transaction containing salt, signerAddress, and data. - /// @return EIP712 hash of the transaction with the domain separator of this contract. - function getTransactionHash(ZeroExTransaction memory transaction, bytes32 eip712ExchangeDomainHash) + /// @dev Calculates the EIP712 typed data hash of a transaction with a given domain separator. + /// @param transaction 0x transaction structure. + /// @return EIP712 typed data hash of the transaction. + function getTypedDataHash(ZeroExTransaction memory transaction, bytes32 eip712ExchangeDomainHash) internal pure returns (bytes32 transactionHash) @@ -55,15 +55,15 @@ library LibZeroExTransaction { // Hash the transaction with the domain separator of the Exchange contract. transactionHash = LibEIP712.hashEIP712Message( eip712ExchangeDomainHash, - transaction.hashZeroExTransaction() + transaction.getStructHash() ); return transactionHash; } - /// @dev Calculates EIP712 hash of the 0x transaction with no domain separator. - /// @param transaction 0x transaction containing salt, signerAddress, and data. - /// @return EIP712 hash of the transaction with no domain separator. - function hashZeroExTransaction(ZeroExTransaction memory transaction) + /// @dev Calculates EIP712 hash of the 0x transaction struct. + /// @param transaction 0x transaction structure. + /// @return EIP712 hash of the transaction struct. + function getStructHash(ZeroExTransaction memory transaction) internal pure returns (bytes32 result) diff --git a/contracts/exchange-libs/contracts/test/TestLibOrder.sol b/contracts/exchange-libs/contracts/test/TestLibOrder.sol index a9e708ac45..c1be49a49b 100644 --- a/contracts/exchange-libs/contracts/test/TestLibOrder.sol +++ b/contracts/exchange-libs/contracts/test/TestLibOrder.sol @@ -24,21 +24,21 @@ import "../src/LibOrder.sol"; contract TestLibOrder { - function getOrderHash(LibOrder.Order memory order, bytes32 eip712ExchangeDomainHash) + function getTypedDataHash(LibOrder.Order memory order, bytes32 eip712ExchangeDomainHash) public pure returns (bytes32 orderHash) { - orderHash = LibOrder.getOrderHash(order, eip712ExchangeDomainHash); + orderHash = LibOrder.getTypedDataHash(order, eip712ExchangeDomainHash); return orderHash; } - function hashOrder(LibOrder.Order memory order) + function getStructHash(LibOrder.Order memory order) public pure returns (bytes32 result) { - result = LibOrder.hashOrder(order); + result = LibOrder.getStructHash(order); return result; } } diff --git a/contracts/exchange-libs/contracts/test/TestLibZeroExTransaction.sol b/contracts/exchange-libs/contracts/test/TestLibZeroExTransaction.sol index 3706d29c94..baaed71877 100644 --- a/contracts/exchange-libs/contracts/test/TestLibZeroExTransaction.sol +++ b/contracts/exchange-libs/contracts/test/TestLibZeroExTransaction.sol @@ -24,21 +24,21 @@ import "../src/LibZeroExTransaction.sol"; contract TestLibZeroExTransaction { - function getZeroExTransactionHash(LibZeroExTransaction.ZeroExTransaction memory transaction, bytes32 eip712ExchangeDomainHash) + function getTypedDataHash(LibZeroExTransaction.ZeroExTransaction memory transaction, bytes32 eip712ExchangeDomainHash) public pure returns (bytes32 transactionHash) { - transactionHash = LibZeroExTransaction.getTransactionHash(transaction, eip712ExchangeDomainHash); + transactionHash = LibZeroExTransaction.getTypedDataHash(transaction, eip712ExchangeDomainHash); return transactionHash; } - function hashZeroExTransaction(LibZeroExTransaction.ZeroExTransaction memory transaction) + function getStructHash(LibZeroExTransaction.ZeroExTransaction memory transaction) public pure returns (bytes32 result) { - result = LibZeroExTransaction.hashZeroExTransaction(transaction); + result = LibZeroExTransaction.getStructHash(transaction); return result; } } \ No newline at end of file diff --git a/contracts/exchange-libs/package.json b/contracts/exchange-libs/package.json index 9939a5fab8..3f093eaa5f 100644 --- a/contracts/exchange-libs/package.json +++ b/contracts/exchange-libs/package.json @@ -34,7 +34,7 @@ "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol" }, "config": { - "abis": "./generated-artifacts/@(LibEIP712ExchangeDomain|LibExchangeRichErrors|LibExchangeSelectors|LibFillResults|LibMath|LibMathRichErrors|LibOrder|LibZeroExTransaction|TestLibEIP712ExchangeDomain|TestLibFillResults|TestLibMath|TestLibOrder|TestLibZeroExTransaction).json", + "abis": "./generated-artifacts/@(LibEIP712ExchangeDomain|LibExchangeRichErrors|LibFillResults|LibMath|LibMathRichErrors|LibOrder|LibZeroExTransaction|TestLibEIP712ExchangeDomain|TestLibFillResults|TestLibMath|TestLibOrder|TestLibZeroExTransaction).json", "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually." }, "repository": { diff --git a/contracts/exchange-libs/src/artifacts.ts b/contracts/exchange-libs/src/artifacts.ts index d4b8c2c5b5..7bfb67a9d3 100644 --- a/contracts/exchange-libs/src/artifacts.ts +++ b/contracts/exchange-libs/src/artifacts.ts @@ -7,7 +7,6 @@ import { ContractArtifact } from 'ethereum-types'; import * as LibEIP712ExchangeDomain from '../generated-artifacts/LibEIP712ExchangeDomain.json'; import * as LibExchangeRichErrors from '../generated-artifacts/LibExchangeRichErrors.json'; -import * as LibExchangeSelectors from '../generated-artifacts/LibExchangeSelectors.json'; import * as LibFillResults from '../generated-artifacts/LibFillResults.json'; import * as LibMath from '../generated-artifacts/LibMath.json'; import * as LibMathRichErrors from '../generated-artifacts/LibMathRichErrors.json'; @@ -21,7 +20,6 @@ import * as TestLibZeroExTransaction from '../generated-artifacts/TestLibZeroExT export const artifacts = { LibEIP712ExchangeDomain: LibEIP712ExchangeDomain as ContractArtifact, LibExchangeRichErrors: LibExchangeRichErrors as ContractArtifact, - LibExchangeSelectors: LibExchangeSelectors as ContractArtifact, LibFillResults: LibFillResults as ContractArtifact, LibMath: LibMath as ContractArtifact, LibMathRichErrors: LibMathRichErrors as ContractArtifact, diff --git a/contracts/exchange-libs/src/wrappers.ts b/contracts/exchange-libs/src/wrappers.ts index a3a7e077ef..89a7ffc03b 100644 --- a/contracts/exchange-libs/src/wrappers.ts +++ b/contracts/exchange-libs/src/wrappers.ts @@ -5,7 +5,6 @@ */ export * from '../generated-wrappers/lib_e_i_p712_exchange_domain'; export * from '../generated-wrappers/lib_exchange_rich_errors'; -export * from '../generated-wrappers/lib_exchange_selectors'; export * from '../generated-wrappers/lib_fill_results'; export * from '../generated-wrappers/lib_math'; export * from '../generated-wrappers/lib_math_rich_errors'; diff --git a/contracts/exchange-libs/test/lib_order.ts b/contracts/exchange-libs/test/lib_order.ts index e1f56a897d..a472bf12c1 100644 --- a/contracts/exchange-libs/test/lib_order.ts +++ b/contracts/exchange-libs/test/lib_order.ts @@ -45,7 +45,7 @@ blockchainTests('LibOrder', env => { version: constants.EIP712_DOMAIN_VERSION, }), ); - const orderHashHex = await libOrderContract.getOrderHash.callAsync(order, domainHash); + const orderHashHex = await libOrderContract.getTypedDataHash.callAsync(order, domainHash); expect(orderHashUtils.getOrderHashHex(order)).to.be.equal(orderHashHex); }); it('orderHash should differ if the domain hash is different', async () => { @@ -64,8 +64,8 @@ blockchainTests('LibOrder', env => { chainId: 1337, }), ); - const orderHashHex1 = await libOrderContract.getOrderHash.callAsync(order, domainHash1); - const orderHashHex2 = await libOrderContract.getOrderHash.callAsync(order, domainHash2); + const orderHashHex1 = await libOrderContract.getTypedDataHash.callAsync(order, domainHash1); + const orderHashHex2 = await libOrderContract.getTypedDataHash.callAsync(order, domainHash2); expect(orderHashHex1).to.be.not.equal(orderHashHex2); }); }); diff --git a/contracts/exchange-libs/test/lib_zero_ex_transaction.ts b/contracts/exchange-libs/test/lib_zero_ex_transaction.ts index 1012097ab7..e5904740dc 100644 --- a/contracts/exchange-libs/test/lib_zero_ex_transaction.ts +++ b/contracts/exchange-libs/test/lib_zero_ex_transaction.ts @@ -38,7 +38,7 @@ blockchainTests('LibZeroExTransaction', env => { version: constants.EIP712_DOMAIN_VERSION, }), ); - const orderHashHex = await libZeroExTransactionContract.getZeroExTransactionHash.callAsync( + const orderHashHex = await libZeroExTransactionContract.getTypedDataHash.callAsync( zeroExTransaction, domainHash, ); @@ -60,11 +60,11 @@ blockchainTests('LibZeroExTransaction', env => { chainId: 1337, }), ); - const transactionHashHex1 = await libZeroExTransactionContract.getZeroExTransactionHash.callAsync( + const transactionHashHex1 = await libZeroExTransactionContract.getTypedDataHash.callAsync( zeroExTransaction, domainHash1, ); - const transactionHashHex2 = await libZeroExTransactionContract.getZeroExTransactionHash.callAsync( + const transactionHashHex2 = await libZeroExTransactionContract.getTypedDataHash.callAsync( zeroExTransaction, domainHash2, ); diff --git a/contracts/exchange-libs/tsconfig.json b/contracts/exchange-libs/tsconfig.json index d2e67a44c2..7e46a4852c 100644 --- a/contracts/exchange-libs/tsconfig.json +++ b/contracts/exchange-libs/tsconfig.json @@ -5,7 +5,6 @@ "files": [ "generated-artifacts/LibEIP712ExchangeDomain.json", "generated-artifacts/LibExchangeRichErrors.json", - "generated-artifacts/LibExchangeSelectors.json", "generated-artifacts/LibFillResults.json", "generated-artifacts/LibMath.json", "generated-artifacts/LibMathRichErrors.json", diff --git a/contracts/exchange/contracts/src/MixinExchangeCore.sol b/contracts/exchange/contracts/src/MixinExchangeCore.sol index e94669e824..20806658c1 100644 --- a/contracts/exchange/contracts/src/MixinExchangeCore.sol +++ b/contracts/exchange/contracts/src/MixinExchangeCore.sol @@ -125,7 +125,7 @@ contract MixinExchangeCore is returns (LibOrder.OrderInfo memory orderInfo) { // Compute the order hash - orderInfo.orderHash = order.getOrderHash(EIP712_EXCHANGE_DOMAIN_HASH); + orderInfo.orderHash = order.getTypedDataHash(EIP712_EXCHANGE_DOMAIN_HASH); // Fetch filled amount orderInfo.orderTakerAssetFilledAmount = filled[orderInfo.orderHash]; diff --git a/contracts/exchange/contracts/src/MixinSignatureValidator.sol b/contracts/exchange/contracts/src/MixinSignatureValidator.sol index c630687a44..d33ea8a406 100644 --- a/contracts/exchange/contracts/src/MixinSignatureValidator.sol +++ b/contracts/exchange/contracts/src/MixinSignatureValidator.sol @@ -137,7 +137,7 @@ contract MixinSignatureValidator is view returns (bool isValid) { - bytes32 orderHash = order.getOrderHash(EIP712_EXCHANGE_DOMAIN_HASH); + bytes32 orderHash = order.getTypedDataHash(EIP712_EXCHANGE_DOMAIN_HASH); return _isValidOrderWithHashSignature( order, orderHash, @@ -157,7 +157,7 @@ contract MixinSignatureValidator is view returns (bool isValid) { - bytes32 transactionHash = transaction.getTransactionHash(EIP712_EXCHANGE_DOMAIN_HASH); + bytes32 transactionHash = transaction.getTypedDataHash(EIP712_EXCHANGE_DOMAIN_HASH); isValid = _isValidTransactionWithHashSignature( transaction, transactionHash, diff --git a/contracts/exchange/contracts/src/MixinTransactions.sol b/contracts/exchange/contracts/src/MixinTransactions.sol index c71be180ca..9958dd6c5b 100644 --- a/contracts/exchange/contracts/src/MixinTransactions.sol +++ b/contracts/exchange/contracts/src/MixinTransactions.sol @@ -85,7 +85,7 @@ contract MixinTransactions is internal returns (bytes memory) { - bytes32 transactionHash = transaction.getTransactionHash(EIP712_EXCHANGE_DOMAIN_HASH); + bytes32 transactionHash = transaction.getTypedDataHash(EIP712_EXCHANGE_DOMAIN_HASH); // Check transaction is not expired // solhint-disable-next-line not-rely-on-time diff --git a/contracts/exchange/contracts/src/interfaces/IExchangeCore.sol b/contracts/exchange/contracts/src/interfaces/IExchangeCore.sol index 085154a7b5..5381623f27 100644 --- a/contracts/exchange/contracts/src/interfaces/IExchangeCore.sol +++ b/contracts/exchange/contracts/src/interfaces/IExchangeCore.sol @@ -32,14 +32,14 @@ contract IExchangeCore { bytes makerAssetData, // Encoded data specific to makerAsset. bytes takerAssetData, // Encoded data specific to takerAsset. bytes makerFeeAssetData, // Encoded data specific to makerFeeAsset. - bytes takerFeeAssetData, // Encoded data specific to takerFeeAsset. + bytes takerFeeAssetData, // Encoded data specific to takerFeeAsset. uint256 makerAssetFilledAmount, // Amount of makerAsset sold by maker and bought by taker. uint256 takerAssetFilledAmount, // Amount of takerAsset sold by taker and bought by maker. uint256 makerFeePaid, // Amount of makerFeeAssetData paid to feeRecipient by maker. uint256 takerFeePaid, // Amount of takerFeeAssetData paid to feeRecipient by taker. address takerAddress, // Address that filled the order. address senderAddress, // Address that called the Exchange contract (msg.sender). - bytes32 indexed orderHash // EIP712 hash of order (see LibOrder.getOrderHash). + bytes32 indexed orderHash // EIP712 hash of order (see LibOrder.getTypedDataHash). ); // Cancel event is emitted whenever an individual order is cancelled. @@ -47,7 +47,7 @@ contract IExchangeCore { address indexed makerAddress, // Address that created the order. address indexed feeRecipientAddress, // Address that would have recieved fees if order was filled. address senderAddress, // Address that called the Exchange contract (msg.sender). - bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash). + bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getTypedDataHash). bytes makerAssetData, // Encoded data specific to makerAsset. bytes takerAssetData // Encoded data specific to takerAsset. ); diff --git a/contracts/exchange/contracts/test/TestExchangeInternals.sol b/contracts/exchange/contracts/test/TestExchangeInternals.sol index fb71b7231d..487b92b74c 100644 --- a/contracts/exchange/contracts/test/TestExchangeInternals.sol +++ b/contracts/exchange/contracts/test/TestExchangeInternals.sol @@ -52,7 +52,7 @@ contract TestExchangeInternals is ) public { - filled[LibOrder.getOrderHash(order, EIP712_EXCHANGE_DOMAIN_HASH)] = orderTakerAssetFilledAmount; + filled[LibOrder.getTypedDataHash(order, EIP712_EXCHANGE_DOMAIN_HASH)] = orderTakerAssetFilledAmount; _updateFilledState( order, takerAddress, diff --git a/contracts/exchange/contracts/test/TestValidatorWallet.sol b/contracts/exchange/contracts/test/TestValidatorWallet.sol index 4be2b08a50..7e0472023e 100644 --- a/contracts/exchange/contracts/test/TestValidatorWallet.sol +++ b/contracts/exchange/contracts/test/TestValidatorWallet.sol @@ -227,7 +227,7 @@ contract TestValidatorWallet is // Use the Exchange to calculate the hash of the order and assert // that it matches the one we extracted previously. require( - LibOrder.getOrderHash(order, _exchange.EIP712_EXCHANGE_DOMAIN_HASH()) == hash, + LibOrder.getTypedDataHash(order, _exchange.EIP712_EXCHANGE_DOMAIN_HASH()) == hash, "UNEXPECTED_ORDER_HASH" ); } else { @@ -238,7 +238,7 @@ contract TestValidatorWallet is // Use the Exchange to calculate the hash of the transaction and assert // that it matches the one we extracted previously. require( - LibZeroExTransaction.getTransactionHash(transaction, _exchange.EIP712_EXCHANGE_DOMAIN_HASH()) == hash, + LibZeroExTransaction.getTypedDataHash(transaction, _exchange.EIP712_EXCHANGE_DOMAIN_HASH()) == hash, "UNEXPECTED_TRANSACTION_HASH" ); } diff --git a/contracts/exchange/contracts/test/TestWrapperFunctions.sol b/contracts/exchange/contracts/test/TestWrapperFunctions.sol index 1befc426b6..598202b12f 100644 --- a/contracts/exchange/contracts/test/TestWrapperFunctions.sol +++ b/contracts/exchange/contracts/test/TestWrapperFunctions.sol @@ -62,7 +62,7 @@ contract TestWrapperFunctions is orderInfo.orderTakerAssetFilledAmount = uint128(order.salt); // High byte of `order.salt` is the `orderStatus`. orderInfo.orderStatus = uint8(order.salt >> 248) % (MAX_ORDER_STATUS + 1); - orderInfo.orderHash = _getOrderHash(order); + orderInfo.orderHash = _getTypedDataHash(order); } /// @dev Overridden to log arguments, be deterministic, and revert with certain inputs. @@ -111,7 +111,7 @@ contract TestWrapperFunctions is } /// @dev Simplified order hashing. - function _getOrderHash(LibOrder.Order memory order) + function _getTypedDataHash(LibOrder.Order memory order) internal pure returns (bytes32 hash)