Putty contest - zer0dot's results

The positionExpirations, exercisedPositons, and positionFloorAssetTokenIds mappings here use uint256 keys when in every use case, the keys are bytes32 cast to uint256. This shouldn't affect gas or anything, but it might make sense just to opt for a bytes32 key, though at times a cast is unavoidable (such as when calling a _mint() or _transferFrom() function.)

In cases where the whitelist could be large, consider sorting whitelisted addresses numerically off-chain and implementing a binary search algorithm for determining whether the filler is whitelisted. (Though this implies an off-chain consensus of whitelist formatting from all apps using Putty.)

In the fillOrder() function in the PuttyV2 contract, there are multiple redundant checks, I'd refactor to branch logic once if the maker order is long, then once again in each branch for whether it's a call. The CEI pattern can still be respected.

Similar to the situation in the case above, the exercise() function in the PuttyV2 contract has a redundant check. I would combine both order.isCall checks to have a more streamlined branching structure.

Similar to fillOrder() and exercise(), there are redundant checks in the withdraw() function in the PuttyV2 contract, I'd combine them into a streamlined branching structure.

In the batchFillOrder() function in the PuttyV2 contract, the two first initial require() statements use the same revert string, combining them with an || operator would likely save code size and possibly gas.

Orders can be cancelled after being filled, this isn't really an issue but should be kept in mind for event consuming applications, as a Cancelled() event will be emitted.

Orders can be cancelled multiple times, this again doesn't really break anything, but it will emit the same event multiple times.

Removing the abi.encodePacked() from constant typehash encoding when the hashing is done on a single string saves minor gas and removes redundant code. See here and here.

Removing the redundant owner check in the exercise() function in the PuttyV2 contract (this is already validated in the call to transferFrom()) saves gas.

Like the point above, removing the redundant owner check in the withdraw() function in the PuttyV2 contract saves gas.

In the withdraw() function in the PuttyV2 contract, refactoring the fee stack variable to only be created if needed saves gas. This comes at the cost of a minor readability degradation though.

Here's the change:

    // Before:
    if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
        // send the fee to the admin/DAO if fee is greater than 0%
        uint256 feeAmount = 0;
        if (fee > 0) {
            feeAmount = (order.strike * fee) / 1000;
            ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
        }

        ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);

        return;
    }

    // After:
    if ((order.isCall && isExercised) || (!order.isCall && !isExercised)) {
        // send the fee to the admin/DAO if fee is greater than 0%
        if (fee > 0) {
            uint256 feeAmount = (order.strike * fee) / 1000;
            ERC20(order.baseAsset).safeTransfer(owner(), feeAmount);
            ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount);
        } else {
            ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike);
        }

        return;
    }