Parse 0x Order in ExchangeWrapper #83
Conversation
| using SafeMath for uint256; | ||
| using LibBytes for bytes; | ||
|
|
||
| /* ============ Constants ============ */ |
There was a problem hiding this comment.
nit: remove empty section headers
|
|
||
|
|
||
| /* ============ State Variables ============ */ | ||
| address public ZERO_EX_EXCHANGE; |
| /* ============ Modifiers ============ */ | ||
|
|
||
|
|
||
| /* ============ Constructor ============ */ |
|
|
||
|
|
||
| /* ============ Public Functions ============ */ | ||
| function exchange( |
| // | | order | 160+signatureLength | orderLength | ZeroEx Order | | ||
|
|
||
| /* | ||
| * Parses the header of |
| let subjectOrderData: Bytes32; | ||
|
|
||
| beforeEach(async () => { | ||
| const zeroExOrderBuffer = bufferZeroExOrder(zeroExOrder); |
There was a problem hiding this comment.
move this setup into test/utils/zeroExExchangeWrapper.ts. give it a descriptive name
| let subjectOrderData: Bytes32; | ||
|
|
||
| beforeEach(async () => { | ||
| const zeroExOrderBuffer = bufferZeroExOrder(zeroExOrder); |
There was a problem hiding this comment.
move this setup into test/utils/zeroExExchangeWrapper.ts. give it a descriptive name
| subjectOrderData = bufferArrayToHex(zeroExOrderBuffer); | ||
| }); | ||
|
|
||
| it("works", async () => { |
There was a problem hiding this comment.
I would really like these to be their own test each, but that might be redundant. try a clever grouping: it parses the correct maker data, it parses the correct taker data...
Pull Request Test Coverage Report for Build 402
💛 - Coveralls |
| }); | ||
|
|
||
| it("should correctly parse the order data header", async () => { | ||
| const result = await zeroExExchangeWrapper.parseOrderDataHeader.callAsync(subjectOrderData); |
| mstore(add(order, 288), mload(add(orderDataAddr, 288))) // salt | ||
| } | ||
|
|
||
| order.makerAssetData = _zeroExOrder.slice(320, _makerAssetDataLength.add(320)); |
There was a problem hiding this comment.
What's 320? can you make it into a constant?
There was a problem hiding this comment.
I'll add / update a table
| }); | ||
|
|
||
| it("should correctly parse the zeroEx order", async () => { | ||
| const result = await zeroExExchangeWrapper.parseZeroExOrderData.callAsync(subjectOrderData); |
There was a problem hiding this comment.
Seems like you don't need this result variable. Assign to the expected outputs. Maybe split up parsing the return variables and do the correct expectations after each one. Or better yet, add a separate spec for each.
| pure | ||
| returns (Order memory) | ||
| { | ||
|
|
| takerAssetData, | ||
| ); | ||
|
|
||
|
|
| Order memory order; | ||
| uint256 orderDataAddr = _zeroExOrder.contentAddress(); | ||
|
|
||
|
|
test/utils/zeroExExchangeWrapper.ts
Outdated
| return ethUtil.setLengthLeft(ethUtil.toBuffer(input), 32); | ||
| } | ||
|
|
||
|
|
| let makerAssetDataLength: BigNumber; | ||
| let takerAssetDataLength: BigNumber; | ||
|
|
||
|
|
No description provided.