diff options
Diffstat (limited to 'packages/contracts')
9 files changed, 112 insertions, 32 deletions
diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 1f5c15674..067a20775 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -62,7 +62,7 @@ "copyfiles": "^1.2.0", "dirty-chai": "^2.0.1", "make-promises-safe": "^1.1.0", - "mocha": "^4.0.1", + "mocha": "^4.1.0", "npm-run-all": "^4.1.2", "shx": "^0.2.2", "solc": "^0.4.24", @@ -86,6 +86,6 @@ "ethereumjs-util": "^5.1.1", "ethers": "3.0.22", "js-combinatorics": "^0.5.3", - "lodash": "^4.17.4" + "lodash": "^4.17.5" } } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol index ac7382715..44de54817 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -96,14 +96,15 @@ contract MixinSignatureValidator is "LENGTH_GREATER_THAN_0_REQUIRED" ); - // Ensure signature is supported + // Pop last byte off of signature byte array. uint8 signatureTypeRaw = uint8(signature.popLastByte()); + + // Ensure signature is supported require( signatureTypeRaw < uint8(SignatureType.NSignatureTypes), "SIGNATURE_UNSUPPORTED" ); - // Pop last byte off of signature byte array. SignatureType signatureType = SignatureType(signatureTypeRaw); // Variables are not scoped in Solidity. @@ -141,7 +142,12 @@ contract MixinSignatureValidator is v = uint8(signature[0]); r = signature.readBytes32(1); s = signature.readBytes32(33); - recovered = ecrecover(hash, v, r, s); + recovered = ecrecover( + hash, + v, + r, + s + ); isValid = signerAddress == recovered; return isValid; @@ -197,7 +203,6 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validatorAddress = signature.popLast20Bytes(); // Ensure signer has approved validator. diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol index b5de1a5de..821d30279 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol @@ -142,7 +142,6 @@ contract MixinTransactions is // Compute hash result := keccak256(memPtr, 128) } - return result; } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol index 704c7061c..4aad37709 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibAbiEncoder.sol @@ -24,20 +24,17 @@ import "./LibOrder.sol"; contract LibAbiEncoder { - /// @dev ABI encodes calldata for `fillOrder` in memory and returns the address range. - /// This range can be passed into `call` or `delegatecall` to invoke an external - /// call to `fillOrder`. + /// @dev ABI encodes calldata for `fillOrder`. /// @param order Order struct containing order specifications. /// @param takerAssetFillAmount Desired amount of takerAsset to sell. /// @param signature Proof that order has been created by maker. - /// @return calldataBegin Memory address of ABI encoded calldata. - /// @return calldataLength Lenfgth of ABI encoded calldata. + /// @return ABI encoded calldata for `fillOrder`. function abiEncodeFillOrder( LibOrder.Order memory order, uint256 takerAssetFillAmount, bytes memory signature ) - public + internal pure returns (bytes memory fillOrderCalldata) { @@ -207,10 +204,10 @@ contract LibAbiEncoder { } // Set length of calldata - mstore( - fillOrderCalldata, - sub(dataAreaEnd, add(fillOrderCalldata, 0x20)) - ) + mstore(fillOrderCalldata, sub(dataAreaEnd, add(fillOrderCalldata, 0x20))) + + // Increment free memory pointer + mstore(0x40, dataAreaEnd) } return fillOrderCalldata; diff --git a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol index 923bac97d..d9cec9edc 100644 --- a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol +++ b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol @@ -22,6 +22,7 @@ pragma experimental ABIEncoderV2; import "../../protocol/Exchange/Exchange.sol"; +// solhint-disable no-empty-blocks contract TestExchangeInternals is Exchange { diff --git a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol index 5a349527b..4a99dd9c1 100644 --- a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol +++ b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol @@ -22,13 +22,33 @@ pragma experimental ABIEncoderV2; import "../../protocol/Exchange/libs/LibMath.sol"; import "../../protocol/Exchange/libs/LibOrder.sol"; import "../../protocol/Exchange/libs/LibFillResults.sol"; +import "../../protocol/Exchange/libs/LibAbiEncoder.sol"; contract TestLibs is LibMath, LibOrder, - LibFillResults + LibFillResults, + LibAbiEncoder { + + function publicAbiEncodeFillOrder( + Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + public + pure + returns (bytes memory fillOrderCalldata) + { + fillOrderCalldata = abiEncodeFillOrder( + order, + takerAssetFillAmount, + signature + ); + return fillOrderCalldata; + } + function publicGetPartialAmount( uint256 numerator, uint256 denominator, diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index f2bb42c75..bef0547bd 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -113,7 +113,7 @@ describe('MixinSignatureValidator', () => { it('should revert when signature type is unsupported', async () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; - const unsupportedSignatureHex = `0x${unsupportedSignatureType}`; + const unsupportedSignatureHex = '0x' + Buffer.from([unsupportedSignatureType]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); return expectContractCallFailed( signatureValidator.publicIsValidSignature.callAsync( @@ -126,7 +126,7 @@ describe('MixinSignatureValidator', () => { }); it('should revert when SignatureType=Illegal', async () => { - const unsupportedSignatureHex = `0x${SignatureType.Illegal}`; + const unsupportedSignatureHex = '0x' + Buffer.from([SignatureType.Illegal]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); return expectContractCallFailed( signatureValidator.publicIsValidSignature.callAsync( @@ -139,7 +139,7 @@ describe('MixinSignatureValidator', () => { }); it('should return false when SignatureType=Invalid and signature has a length of zero', async () => { - const signatureHex = `0x${SignatureType.Invalid}`; + const signatureHex = '0x' + Buffer.from([SignatureType.Invalid]).toString('hex'); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const isValidSignature = await signatureValidator.publicIsValidSignature.callAsync( orderHashHex, diff --git a/packages/contracts/test/utils/exchange_wrapper.ts b/packages/contracts/test/utils/exchange_wrapper.ts index 490ea959b..d57592d6d 100644 --- a/packages/contracts/test/utils/exchange_wrapper.ts +++ b/packages/contracts/test/utils/exchange_wrapper.ts @@ -8,7 +8,7 @@ import { ExchangeContract } from '../../generated_contract_wrappers/exchange'; import { formatters } from './formatters'; import { LogDecoder } from './log_decoder'; import { orderUtils } from './order_utils'; -import { OrderInfo, SignedTransaction } from './types'; +import { FillResults, OrderInfo, SignedTransaction } from './types'; export class ExchangeWrapper { private readonly _exchange: ExchangeContract; @@ -243,4 +243,27 @@ export class ExchangeWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } + public async getFillOrderResultsAsync( + signedOrder: SignedOrder, + from: string, + opts: { takerAssetFillAmount?: BigNumber } = {}, + ): Promise<FillResults> { + const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); + const fillResults = await this._exchange.fillOrder.callAsync( + params.order, + params.takerAssetFillAmount, + params.signature, + { from }, + ); + return fillResults; + } + public abiEncodeFillOrder(signedOrder: SignedOrder, opts: { takerAssetFillAmount?: BigNumber } = {}): string { + const params = orderUtils.createFill(signedOrder, opts.takerAssetFillAmount); + const data = this._exchange.fillOrder.getABIEncodedTransactionData( + params.order, + params.takerAssetFillAmount, + params.signature, + ); + return data; + } } diff --git a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts index bdd1e62a2..284c4a2db 100644 --- a/packages/contracts/test/utils/fill_order_combinatorial_utils.ts +++ b/packages/contracts/test/utils/fill_order_combinatorial_utils.ts @@ -15,6 +15,7 @@ import * as _ from 'lodash'; import 'make-promises-safe'; import { ExchangeContract, ExchangeFillEventArgs } from '../../generated_contract_wrappers/exchange'; +import { TestLibsContract } from '../../generated_contract_wrappers/test_libs'; import { artifacts } from './artifacts'; import { expectTransactionFailedAsync } from './assertions'; @@ -123,6 +124,8 @@ export async function fillOrderCombinatorialUtilsFactoryAsync( exchangeContract.address, ); + const testLibsContract = await TestLibsContract.deployFrom0xArtifactAsync(artifacts.TestLibs, provider, txDefaults); + const fillOrderCombinatorialUtils = new FillOrderCombinatorialUtils( orderFactory, ownerAddress, @@ -132,6 +135,7 @@ export async function fillOrderCombinatorialUtilsFactoryAsync( zrxAssetData, exchangeWrapper, assetWrapper, + testLibsContract, ); return fillOrderCombinatorialUtils; } @@ -145,6 +149,7 @@ export class FillOrderCombinatorialUtils { public zrxAssetData: string; public exchangeWrapper: ExchangeWrapper; public assetWrapper: AssetWrapper; + public testLibsContract: TestLibsContract; public static generateFillOrderCombinations(): FillScenario[] { const takerScenarios = [ TakerScenario.Unspecified, @@ -329,6 +334,7 @@ export class FillOrderCombinatorialUtils { zrxAssetData: string, exchangeWrapper: ExchangeWrapper, assetWrapper: AssetWrapper, + testLibsContract: TestLibsContract, ) { this.orderFactory = orderFactory; this.ownerAddress = ownerAddress; @@ -338,6 +344,7 @@ export class FillOrderCombinatorialUtils { this.zrxAssetData = zrxAssetData; this.exchangeWrapper = exchangeWrapper; this.assetWrapper = assetWrapper; + this.testLibsContract = testLibsContract; } public async testFillOrderScenarioAsync( provider: Provider, @@ -410,6 +417,8 @@ export class FillOrderCombinatorialUtils { lazyStore, fillRevertReasonIfExists, ); + + await this._abiEncodeFillOrderAndAssertOutcomeAsync(signedOrder, takerAssetFillAmount); } private async _fillOrderAndAssertOutcomeAsync( signedOrder: SignedOrder, @@ -456,6 +465,29 @@ export class FillOrderCombinatorialUtils { signedOrder.takerAssetAmount, signedOrder.makerAssetAmount, ); + const expMakerFeePaid = orderUtils.getPartialAmount( + expFilledTakerAmount, + signedOrder.takerAssetAmount, + signedOrder.makerFee, + ); + const expTakerFeePaid = orderUtils.getPartialAmount( + expFilledTakerAmount, + signedOrder.takerAssetAmount, + signedOrder.takerFee, + ); + const fillResults = await this.exchangeWrapper.getFillOrderResultsAsync(signedOrder, this.takerAddress, { + takerAssetFillAmount, + }); + expect(fillResults.takerAssetFilledAmount).to.be.bignumber.equal( + expFilledTakerAmount, + 'takerAssetFilledAmount', + ); + expect(fillResults.makerAssetFilledAmount).to.be.bignumber.equal( + expFilledMakerAmount, + 'makerAssetFilledAmount', + ); + expect(fillResults.takerFeePaid).to.be.bignumber.equal(expTakerFeePaid, 'takerFeePaid'); + expect(fillResults.makerFeePaid).to.be.bignumber.equal(expMakerFeePaid, 'makerFeePaid'); // - Let's fill the order! const txReceipt = await this.exchangeWrapper.fillOrderAsync(signedOrder, this.takerAddress, { @@ -479,17 +511,7 @@ export class FillOrderCombinatorialUtils { expFilledTakerAmount, 'log.args.takerAssetFilledAmount', ); - const expMakerFeePaid = orderUtils.getPartialAmount( - expFilledTakerAmount, - signedOrder.takerAssetAmount, - signedOrder.makerFee, - ); expect(log.args.makerFeePaid).to.be.bignumber.equal(expMakerFeePaid, 'log.args.makerFeePaid'); - const expTakerFeePaid = orderUtils.getPartialAmount( - expFilledTakerAmount, - signedOrder.takerAssetAmount, - signedOrder.takerFee, - ); expect(log.args.takerFeePaid).to.be.bignumber.equal(expTakerFeePaid, 'logs.args.takerFeePaid'); expect(log.args.orderHash).to.be.equal(orderHash, 'log.args.orderHash'); expect(log.args.makerAssetData).to.be.equal(makerAssetData, 'log.args.makerAssetData'); @@ -571,6 +593,19 @@ export class FillOrderCombinatorialUtils { 'ZRXAssetBalanceOfFeeRecipient', ); } + private async _abiEncodeFillOrderAndAssertOutcomeAsync( + signedOrder: SignedOrder, + takerAssetFillAmount: BigNumber, + ): Promise<void> { + const params = orderUtils.createFill(signedOrder, takerAssetFillAmount); + const expectedAbiEncodedData = this.exchangeWrapper.abiEncodeFillOrder(signedOrder, { takerAssetFillAmount }); + const libsAbiEncodedData = await this.testLibsContract.publicAbiEncodeFillOrder.callAsync( + params.order, + params.takerAssetFillAmount, + params.signature, + ); + expect(libsAbiEncodedData).to.be.equal(expectedAbiEncodedData, 'ABIEncodedFillOrderData'); + } private async _getTakerAssetFillAmountAsync( signedOrder: SignedOrder, takerAssetFillAmountScenario: TakerAssetFillAmountScenario, |