Conversation
mattdf
commented
Apr 27, 2018
mattdf
commented
- extend trade func to return fill amounts
- implement fill per order and cumulative fill checks
- write tests
contracts/Exchange.sol
Outdated
| } | ||
|
|
||
|
|
||
| function multitrade(uint numOrders, address[] addresses, uint[] values, bytes32[] sigmain, uint16[] sigaux, uint maxFillAmount) external { |
There was a problem hiding this comment.
We should look into this because of the trade off of readibility.
contracts/Exchange.sol
Outdated
|
|
||
| address[3] memory addrs; | ||
| uint[4] memory vals; | ||
| bytes memory s = new bytes(66); |
| bytes32 s1 = sm[i*2]; | ||
| bytes32 s2 = sm[i*2 + 1]; | ||
| uint16 s3 = sa[i]; | ||
| bytes memory s = new bytes(66); |
There was a problem hiding this comment.
Check that not everything is 0x0 before, if it is, do a return.
contracts/Exchange.sol
Outdated
| require(addresses.length == 3*numOrders); | ||
| require(values.length == 4*numOrders); | ||
| require(sigmain.length == 2*numOrders); | ||
| require(sigaux.length == numOrders); |
There was a problem hiding this comment.
numOrders is not needed, as we can guarantee that sigaux is the length of numorders.
contracts/Exchange.sol
Outdated
| vals[j] = values[(i*4)+j]; | ||
| } | ||
| s = sigArrayToBytes(sigmain, sigaux, i); | ||
| trade(OrderLibrary.createOrder(addrs, vals), msg.sender, s, maxFillAmount); |
There was a problem hiding this comment.
We should consider doing this differently, maybe first checking if we can trade. Otherwise this function will throw, which might be pretty stupid as that will cancel the entire multitrade
* Created library * removed unused imports * renamed
| uint maxFillAmount | ||
| ) | ||
| internal | ||
| internal returns (uint) |
| } | ||
| addrs[0] = makers[i]; | ||
| addrs[1] = makerToken; | ||
| addrs[2] = takerToken; |
| vals[j] = values[(i*4)+j]; | ||
| } | ||
| s = sigArrayToBytes(sigmain, sigaux, i); | ||
| exchange.trade(OrderLibrary.createOrder(addrs, vals), msg.sender, s, maxFillAmount[i]); |
There was a problem hiding this comment.
no need for s, do it right here.
| } | ||
|
|
||
|
|
||
| function multitrade(address[] addresses, uint[] values, bytes32[] sigmain, uint16[] sigaux, uint[] maxFillAmount) external { |
There was a problem hiding this comment.
Even if we do not fully use it, we should consider ABI Encoder V2 here, it will allow us to use an array of bytes, rather than this weird split we are doing here.
2 participants
