From 2f639b77bbcad9884b25536787e1d35a2eee2e34 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 30 Mar 2018 11:05:35 -0700 Subject: Update Solidity syntax and comments --- .../current/protocol/Exchange/LibErrors.sol | 2 +- .../protocol/Exchange/MixinExchangeCore.sol | 33 +++++------ .../protocol/Exchange/MixinWrapperFunctions.sol | 67 +++++++++++----------- packages/contracts/test/exchange/core.ts | 28 ++++----- packages/contracts/test/exchange/wrapper.ts | 7 +-- 5 files changed, 67 insertions(+), 70 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol index 7fa9d4860..55160ad5b 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/LibErrors.sol @@ -30,6 +30,6 @@ contract LibErrors { INSUFFICIENT_BALANCE_OR_ALLOWANCE // Insufficient balance or allowance for token transfer } - event LogError(uint8 indexed errorId, bytes32 indexed orderHash); + event ExchangeError(uint8 indexed errorId, bytes32 indexed orderHash); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index aefffe465..ea1cf507b 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -49,20 +49,20 @@ contract MixinExchangeCore is // Orders with a salt less than their maker's epoch are considered cancelled mapping (address => uint256) public makerEpoch; - event LogFill( + event Fill( address indexed makerAddress, address takerAddress, address indexed feeRecipientAddress, address makerTokenAddress, address takerTokenAddress, uint256 makerAmountSold, - uint256 makerAmountBought, + uint256 takerAmountSold, uint256 makerFeePaid, uint256 takerFeePaid, bytes32 indexed orderHash ); - event LogCancel( + event Cancel( address indexed makerAddress, address indexed feeRecipientAddress, address makerTokenAddress, @@ -70,7 +70,7 @@ contract MixinExchangeCore is bytes32 indexed orderHash ); - event LogCancelUpTo( + event CancelUpTo( address indexed makerAddress, uint256 makerEpoch ); @@ -82,8 +82,8 @@ contract MixinExchangeCore is /// @dev Fills the input order. /// @param order Order struct containing order specifications. /// @param takerSellAmount Desired amount of takerToken to sell. - /// @param signature Proof of signing order by maker. - /// @return Total amount of takerToken filled in trade. + /// @param signature Proof that order has been created by maker. + /// @return Amounts filled and fees paid by maker and taker. function fillOrder( Order memory order, uint256 takerSellAmount, @@ -102,7 +102,7 @@ contract MixinExchangeCore is // Check if order has been cancelled by orderHash if (cancelled[orderHash]) { - LogError(uint8(Errors.ORDER_CANCELLED), orderHash); + emit ExchangeError(uint8(Errors.ORDER_CANCELLED), orderHash); return fillResults; } @@ -119,21 +119,21 @@ contract MixinExchangeCore is // Validate order expiration if (block.timestamp >= order.expirationTimeSeconds) { - LogError(uint8(Errors.ORDER_EXPIRED), orderHash); + emit ExchangeError(uint8(Errors.ORDER_EXPIRED), orderHash); return fillResults; } // Validate order availability uint256 remainingMakerBuyAmount = safeSub(order.makerBuyAmount, filled[orderHash]); if (remainingMakerBuyAmount == 0) { - LogError(uint8(Errors.ORDER_FULLY_FILLED), orderHash); + emit ExchangeError(uint8(Errors.ORDER_FULLY_FILLED), orderHash); return fillResults; } // Validate fill order rounding fillResults.takerAmountSold = min256(takerSellAmount, remainingMakerBuyAmount); if (isRoundingError(fillResults.takerAmountSold, order.makerBuyAmount, order.makerSellAmount)) { - LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash); + emit ExchangeError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash); fillResults.takerAmountSold = 0; return fillResults; } @@ -146,7 +146,7 @@ contract MixinExchangeCore is settleOrder(order, msg.sender, fillResults.takerAmountSold); // Log order - LogFill( + emit Fill( order.makerAddress, msg.sender, order.feeRecipientAddress, @@ -163,7 +163,8 @@ contract MixinExchangeCore is /// @dev After calling, the order can not be filled anymore. /// @param order Order struct containing order specifications. - /// @return True if the order state changed to cancelled. False if the transaction was already cancelled or expired. + /// @return True if the order state changed to cancelled. + /// False if the transaction was already cancelled or expired. function cancelOrder(Order memory order) public returns (bool) @@ -175,18 +176,18 @@ contract MixinExchangeCore is require(order.makerAddress == msg.sender); if (block.timestamp >= order.expirationTimeSeconds) { - LogError(uint8(Errors.ORDER_EXPIRED), orderHash); + emit ExchangeError(uint8(Errors.ORDER_EXPIRED), orderHash); return false; } if (cancelled[orderHash]) { - LogError(uint8(Errors.ORDER_CANCELLED), orderHash); + emit ExchangeError(uint8(Errors.ORDER_CANCELLED), orderHash); return false; } cancelled[orderHash] = true; - LogCancel( + emit Cancel( order.makerAddress, order.feeRecipientAddress, order.makerTokenAddress, @@ -203,7 +204,7 @@ contract MixinExchangeCore is uint256 newMakerEpoch = salt + 1; // makerEpoch is initialized to 0, so to cancelUpTo we need salt+1 require(newMakerEpoch > makerEpoch[msg.sender]); // epoch must be monotonically increasing makerEpoch[msg.sender] = newMakerEpoch; - LogCancelUpTo(msg.sender, newMakerEpoch); + emit CancelUpTo(msg.sender, newMakerEpoch); } /// @dev Checks if rounding error > 0.1%. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 34bd77092..88505ef0e 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -31,8 +31,8 @@ contract MixinWrapperFunctions is { /// @dev Fills the input order. Reverts if exact takerSellAmount not filled. /// @param order Order struct containing order specifications. - /// @param takerSellAmount Desired amount of takerToken to fill. - /// @param signature Maker's signature of the order. + /// @param takerSellAmount Desired amount of takerToken to sell. + /// @param signature Proof that order has been created by maker. function fillOrKillOrder( Order memory order, uint256 takerSellAmount, @@ -52,8 +52,8 @@ contract MixinWrapperFunctions is /// @dev Fills an order with specified parameters and ECDSA signature. /// Returns false if the transaction would otherwise revert. /// @param order Order struct containing order specifications. - /// @param takerSellAmount Desired amount of takerToken to fill. - /// @param signature Maker's signature of the order. + /// @param takerSellAmount Desired amount of takerToken to sell. + /// @param signature Proof that order has been created by maker. /// @return Amounts filled and fees paid by maker and taker. function fillOrderNoThrow( Order memory order, @@ -154,10 +154,10 @@ contract MixinWrapperFunctions is return fillResults; } - /// @dev Synchronously executes multiple calls of fillOrder in a single transaction. - /// @param orders Array of orders. - /// @param takerSellAmounts Array of desired amounts of takerToken to fill in orders. - /// @param signatures Maker's signatures of the orders. + /// @dev Synchronously executes multiple calls of fillOrder. + /// @param orders Array of order specifications. + /// @param takerSellAmounts Array of desired amounts of takerToken to sell in orders. + /// @param signatures Proofs that orders have been created by makers. function batchFillOrders( Order[] memory orders, uint256[] memory takerSellAmounts, @@ -173,10 +173,10 @@ contract MixinWrapperFunctions is } } - /// @dev Synchronously executes multiple calls of fillOrKill in a single transaction. - /// @param orders Array of orders. - /// @param takerSellAmounts Array of desired amounts of takerToken to fill in orders. - /// @param signatures Maker's signatures of the orders. + /// @dev Synchronously executes multiple calls of fillOrKill. + /// @param orders Array of order specifications. + /// @param takerSellAmounts Array of desired amounts of takerToken to sell in orders. + /// @param signatures Proofs that orders have been created by makers. function batchFillOrKillOrders( Order[] memory orders, uint256[] memory takerSellAmounts, @@ -192,10 +192,11 @@ contract MixinWrapperFunctions is } } - /// @dev Fills an order with specified parameters and ECDSA signature. Returns false if the transaction would otherwise revert. - /// @param orders Array of orders. - /// @param takerSellAmounts Array of desired amounts of takerToken to fill in orders. - /// @param signatures Maker's signatures of the orders. + /// @dev Fills an order with specified parameters and ECDSA signature. + /// Returns false if the transaction would otherwise revert. + /// @param orders Array of order specifications. + /// @param takerSellAmounts Array of desired amounts of takerToken to sell in orders. + /// @param signatures Proofs that orders have been created by makers. function batchFillOrdersNoThrow( Order[] memory orders, uint256[] memory takerSellAmounts, @@ -211,11 +212,11 @@ contract MixinWrapperFunctions is } } - /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is sold by taker. - /// @param orders Array of orders. + /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerToken is sold by taker. + /// @param orders Array of order specifications. /// @param takerSellAmount Desired amount of takerToken to sell. - /// @param signatures Maker's signatures of the orders. - /// @return Total amount of tokens sold by taker in orders. + /// @param signatures Proofs that orders have been created by makers. + /// @return Amounts filled and fees paid by makers and taker. function marketSellOrders( Order[] memory orders, uint256 takerSellAmount, @@ -242,12 +243,12 @@ contract MixinWrapperFunctions is return fillResults; } - /// @dev Synchronously executes multiple calls of fillOrderNoThrow in a single transaction until total amount is sold by taker. + /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerToken is sold by taker. /// Returns false if the transaction would otherwise revert. - /// @param orders Array of orders. + /// @param orders Array of order specifications. /// @param takerSellAmount Desired amount of takerToken to sell. - /// @param signatures Maker's signatures of the orders. - /// @return Total amount of tokens sold by taker in orders. + /// @param signatures Proofs that orders have been signed by makers. + /// @return Amounts filled and fees paid by makers and taker. function marketSellOrdersNoThrow( Order[] memory orders, uint256 takerSellAmount, @@ -274,11 +275,11 @@ contract MixinWrapperFunctions is return fillResults; } - /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker. - /// @param orders Array of orders. + /// @dev Synchronously executes multiple calls of fillOrder until total amount of makerToken is bought by taker. + /// @param orders Array of order specifications. /// @param takerBuyAmount Desired amount of makerToken to buy. - /// @param signatures Maker's signatures of the orders. - /// @return Total amount of takerTokenFillAmount filled in orders. + /// @param signatures Proofs that orders have been signed by makers. + /// @return Amounts filled and fees paid by makers and taker. function marketBuyOrders( Order[] memory orders, uint256 takerBuyAmount, @@ -312,10 +313,10 @@ contract MixinWrapperFunctions is /// @dev Synchronously executes multiple fill orders in a single transaction until total amount is bought by taker. /// Returns false if the transaction would otherwise revert. - /// @param orders Array of orders. - /// @param takerBuyAmount Desired amount of makerToken to fill. - /// @param signatures Maker's signatures of the orders. - /// @return Total amount of takerTokenFillAmount filled in orders. + /// @param orders Array of order specifications. + /// @param takerBuyAmount Desired amount of makerToken to buy. + /// @param signatures Proofs that orders have been signed by makers. + /// @return Amounts filled and fees paid by makers and taker. function marketBuyOrdersNoThrow( Order[] memory orders, uint256 takerBuyAmount, @@ -348,7 +349,7 @@ contract MixinWrapperFunctions is } /// @dev Synchronously cancels multiple orders in a single transaction. - /// @param orders Array of orders. + /// @param orders Array of order specifications. function batchCancelOrders(Order[] memory orders) public { diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index b9ad309fc..406a96fa0 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -9,10 +9,10 @@ import * as Web3 from 'web3'; import { DummyTokenContract } from '../../src/contract_wrappers/generated/dummy_token'; import { + CancelContractEventArgs, ExchangeContract, - LogCancelContractEventArgs, - LogErrorContractEventArgs, - LogFillContractEventArgs, + ExchangeErrorContractEventArgs, + FillContractEventArgs, } from '../../src/contract_wrappers/generated/exchange'; import { TokenTransferProxyContract } from '../../src/contract_wrappers/generated/token_transfer_proxy'; import { Balances } from '../../src/utils/balances'; @@ -394,8 +394,8 @@ describe('Exchange', () => { const res = await exWrapper.fillOrderAsync(signedOrder, takerAddress, { takerSellAmount: signedOrder.makerBuyAmount, }); - const log = res.logs[0] as LogWithDecodedArgs; - expect(log.args.makerAmountBought).to.be.bignumber.equal(signedOrder.makerBuyAmount.minus(takerSellAmount)); + const log = res.logs[0] as LogWithDecodedArgs; + expect(log.args.takerAmountSold).to.be.bignumber.equal(signedOrder.makerBuyAmount.minus(takerSellAmount)); const newBalances = await dmyBalances.getAsync(); expect(newBalances[makerAddress][signedOrder.makerTokenAddress]).to.be.bignumber.equal( @@ -428,7 +428,7 @@ describe('Exchange', () => { }); expect(res.logs).to.have.length(1); - const log = res.logs[0] as LogWithDecodedArgs; + const log = res.logs[0] as LogWithDecodedArgs; const logArgs = log.args; const expectedFilledMakerTokenAmount = signedOrder.makerSellAmount.div(divisor); const expectedFilledTakerTokenAmount = signedOrder.makerBuyAmount.div(divisor); @@ -441,7 +441,7 @@ describe('Exchange', () => { expect(signedOrder.makerTokenAddress).to.be.equal(logArgs.makerTokenAddress); expect(signedOrder.takerTokenAddress).to.be.equal(logArgs.takerTokenAddress); expect(expectedFilledMakerTokenAmount).to.be.bignumber.equal(logArgs.makerAmountSold); - expect(expectedFilledTakerTokenAmount).to.be.bignumber.equal(logArgs.makerAmountBought); + expect(expectedFilledTakerTokenAmount).to.be.bignumber.equal(logArgs.takerAmountSold); expect(expectedFeeMPaid).to.be.bignumber.equal(logArgs.makerFeePaid); expect(expectedFeeTPaid).to.be.bignumber.equal(logArgs.takerFeePaid); expect(orderUtils.getOrderHashHex(signedOrder)).to.be.equal(logArgs.orderHash); @@ -457,7 +457,7 @@ describe('Exchange', () => { }); expect(res.logs).to.have.length(1); - const log = res.logs[0] as LogWithDecodedArgs; + const log = res.logs[0] as LogWithDecodedArgs; const logArgs = log.args; const expectedFilledMakerTokenAmount = signedOrder.makerSellAmount.div(divisor); const expectedFilledTakerTokenAmount = signedOrder.makerBuyAmount.div(divisor); @@ -470,7 +470,7 @@ describe('Exchange', () => { expect(signedOrder.makerTokenAddress).to.be.equal(logArgs.makerTokenAddress); expect(signedOrder.takerTokenAddress).to.be.equal(logArgs.takerTokenAddress); expect(expectedFilledMakerTokenAmount).to.be.bignumber.equal(logArgs.makerAmountSold); - expect(expectedFilledTakerTokenAmount).to.be.bignumber.equal(logArgs.makerAmountBought); + expect(expectedFilledTakerTokenAmount).to.be.bignumber.equal(logArgs.takerAmountSold); expect(expectedFeeMPaid).to.be.bignumber.equal(logArgs.makerFeePaid); expect(expectedFeeTPaid).to.be.bignumber.equal(logArgs.takerFeePaid); expect(orderUtils.getOrderHashHex(signedOrder)).to.be.equal(logArgs.orderHash); @@ -552,7 +552,7 @@ describe('Exchange', () => { const res = await exWrapper.fillOrderAsync(signedOrder, takerAddress); expect(res.logs).to.have.length(1); - const log = res.logs[0] as LogWithDecodedArgs; + const log = res.logs[0] as LogWithDecodedArgs; const errCode = log.args.errorId; expect(errCode).to.be.equal(ExchangeContractErrs.ERROR_ORDER_EXPIRED); }); @@ -563,7 +563,7 @@ describe('Exchange', () => { const res = await exWrapper.fillOrderAsync(signedOrder, takerAddress); expect(res.logs).to.have.length(1); - const log = res.logs[0] as LogWithDecodedArgs; + const log = res.logs[0] as LogWithDecodedArgs; const errCode = log.args.errorId; expect(errCode).to.be.equal(ExchangeContractErrs.ERROR_ORDER_FULLY_FILLED); }); @@ -594,7 +594,7 @@ describe('Exchange', () => { const res = await exWrapper.cancelOrderAsync(signedOrder, makerAddress); expect(res.logs).to.have.length(1); - const log = res.logs[0] as LogWithDecodedArgs; + const log = res.logs[0] as LogWithDecodedArgs; const logArgs = log.args; expect(signedOrder.makerAddress).to.be.equal(logArgs.makerAddress); @@ -609,7 +609,7 @@ describe('Exchange', () => { const res = await exWrapper.cancelOrderAsync(signedOrder, makerAddress); expect(res.logs).to.have.length(1); - const log = res.logs[0] as LogWithDecodedArgs; + const log = res.logs[0] as LogWithDecodedArgs; const errCode = log.args.errorId; expect(errCode).to.be.equal(ExchangeContractErrs.ERROR_ORDER_CANCELLED); }); @@ -621,7 +621,7 @@ describe('Exchange', () => { const res = await exWrapper.cancelOrderAsync(signedOrder, makerAddress); expect(res.logs).to.have.length(1); - const log = res.logs[0] as LogWithDecodedArgs; + const log = res.logs[0] as LogWithDecodedArgs; const errCode = log.args.errorId; expect(errCode).to.be.equal(ExchangeContractErrs.ERROR_ORDER_EXPIRED); }); diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index 295d5b204..145d9ad55 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -7,12 +7,7 @@ import * as _ from 'lodash'; import * as Web3 from 'web3'; import { DummyTokenContract } from '../../src/contract_wrappers/generated/dummy_token'; -import { - ExchangeContract, - LogCancelContractEventArgs, - LogErrorContractEventArgs, - LogFillContractEventArgs, -} from '../../src/contract_wrappers/generated/exchange'; +import { ExchangeContract } from '../../src/contract_wrappers/generated/exchange'; import { TokenRegistryContract } from '../../src/contract_wrappers/generated/token_registry'; import { TokenTransferProxyContract } from '../../src/contract_wrappers/generated/token_transfer_proxy'; import { Balances } from '../../src/utils/balances'; -- cgit v1.2.3