Putty contest - rokinot's results

Summary

A single low / non-critical vulnerability was found.

cancel() can cancel the same order multiple times

L#526

Impact

Users could mistakenly cancel their orders multiple times, wasting funds in the meantime.

Recommended Mitigation Steps

A simple fix is adding a requirement to check if the order has been cancelled already, as seen below. The off-chain provider could also possibly mitigate this issue on their front end, although this is unknown.

require(msg.sender == order.maker, "Not your order");

bytes32 orderHash = hashOrder(order);

require(cancelledOrders[orderHash], "Order already cancelled");

// mark the order as cancelled
cancelledOrders[orderHash] = true;

emit CancelledOrder(orderHash, order);

Constant variables only used inside the scope of the contract should be downgraded from public to private.

#L90 #L95 #L101

setBaseURI() and setFee() can be inlined in the constructor in order to remove JUMP opcodes, decreasing gas.

#L209-219

setBaseURI(_baseURI); setFee(_fee); weth = _weth;

<= is cheaper than < due to lower checks. this optimization will keep the contract's logic intact.

#L287

L#498

In this case, >=1 is functionally identical to >0.

!= 0 checks are cheaper than > 0 checks with the optimizer on.

#L293

require() becomes cheaper if the error message has less than 32 characters.

L#398 L#401

e.g. "Must be a long position"

Do not initialize variables if you will assign them as 0 since they start at 0 by default.

L#497 L#594 L#611 L#627 L#637 L#647 L#658 L#670 L#728 L#742

<array>.length should be cached

L#551 L#594 L#611 L#627 L#637 L#647 L#658 L#670 L#728 L#742

the loop iterator ++i is cheaper than i++

L#594 L#611 L#627 L#637 L#647 L#658 L#670 L#728 L#742

iterators should be left unchecked as they have no risk of overflowing.

L#594 L#611 L#627 L#637 L#647 L#658 L#670 L#728 L#742

!= comparisons for uint will consume less gas than >

L#293 L#327 L#351 L#427 L#598 L#599