Putty contest - joestakey's results

QA Report

Table of Contents

• summary
• Hash collision with abi.encodePacked
• _mint does not check if recipient can handle tokens
• Payable functions when using ERC20
• Constants instead of magic numbers
• Events emitted early
• Events indexing
• Function order
• Public functions can be external
• Related data should be grouped in struct
• Scientific notation
• Timelock on setters
• TransferOwnership can be 2-steps
• Unused returns

summary

Few vulnerabilities were found. The main concerns are with:

Hash collision with abi.encodePacked

IMPACT

strings and bytes are encoded with padding when using abi.encodePacked. This can lead to hash collision when passing the result to keccak256

SEVERITY

Low

PROOF OF CONCEPT

Two instances where abi.encodePacked is used with bytes arguments:

PuttyV2.sol

encoded = abi.encodePacked(encoded,keccak256(abi.encode(ERC20ASSET_TYPE_HASH, arr[i].token, arr[i].tokenAmount)))
encoded = abi.encodePacked(encoded,keccak256(abi.encode(ERC721ASSET_TYPE_HASH, arr[i].token, arr[i].tokenId)))

TOOLS USED

Manual Analysis

MITIGATION

Use abi.encode() instead.

_mint does not check if recipient can handle tokens

PROBLEM

_mint() defined in PuttyV2Nft.sol does not checks whether to can handle ERC721 tokens if it is not an EOA. The function is called in PuttyV2.fillOrder to mint the tokens representing the long and short position for the order. If either order.maker or the caller is a contract not handling ERC721 tokens, one of the minted tokens will be lost, which can result in either exercise or withdraw not being able to be called

SEVERITY

Low

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

scope: fillOrder

_mint(order.maker, uint256(orderHash))
_mint(msg.sender, positionId)

TOOLS USED

Manual Analysis

MITIGATION

Add a check in _mint to ensure to implements an ERC721Receiver if it is not an EOA

Payable functions when using ERC20

PROBLEM

There should be a require(0 == msg.value) to ensure no Ether is being sent to PuttyV2.sol when the order.baseAsset used in an order is a ERC20 token. As there is no way to withdraw ETH from the contract, any ETH mistakenly sent when order.baseAsset is an ERC20 token will result in locked ETH in the contract.

SEVERITY

Low

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

scope: fillOrder

• When order.baseAsset is an ERC20 token, there is no check to see if msg.value == 0. Same issue here

scope: exercise

• When order.baseAsset is an ERC20 token, there is no check to see if msg.value == 0. Same issue here

TOOLS USED

Manual Analysis

MITIGATION

Add require(0 == msg.value) in all blocks mentioned above.

Constants instead of magic numbers

PROBLEM

It is best practice to use constant variables rather than literal values to make the code easier to understand and maintain.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

require(_fee < 30, "fee must be less than 3%")
require(order.duration < 10_000 days, "Duration too long")
feeAmount = (order.strike * fee) / 1000

TOOLS USED

Manual Analysis

MITIGATION

Define constant variables for the literal values aforementioned.

Events emitted early

PROBLEM

Events should be emitted at the end of a function call, to prevent emission if the function reverts.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

All the following events are emitted before external calls are performed.

emit FilledOrder(orderHash, floorAssetTokenIds, order)
emit ExercisedOrder(orderHash, floorAssetTokenIds, order)
emit WithdrawOrder(orderHash, order)

TOOLS USED

Manual Analysis

MITIGATION

Emit events at the end of functions.

Events indexing

PROBLEM

Events should use indexed fields, to make them easier to filter in logs.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

event NewBaseURI(string baseURI)
event NewFee(uint256 fee)

TOOLS USED

Manual Analysis

MITIGATION

Add indexed fields to these events.

Function order

PROBLEM

Functions should be ordered following the Soldiity conventions. This means ordering functions based on their visibility.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Several contracts have public functions defined after internal functions:

• PuttyV2.sol

• PuttyV2Nft.sol

TOOLS USED

Manual Analysis

MITIGATION

Place the public functions internal functions.

`

Public functions can be external

PROBLEM

It is good practice to mark functions as external instead of public if they are not called by the contract where they are defined.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

function exercise()
function withdraw()
function batchFillOrder()
function acceptCounterOffer()
function domainSeparatorV4()

TOOLS USED

Manual Analysis

MITIGATION

Declare this function as external instead of public

Related data should be grouped in struct

PROBLEM

When there are mappings that use the same key value, having separate fields is error prone, for instance in case of deletion or with future new fields.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

mapping(uint256 => uint256) public positionExpirations
mapping(uint256 => bool) public exercisedPositions
mapping(uint256 => uint256[]) public positionFloorAssetTokenIds

TOOLS USED

Manual Analysis

MITIGATION

Group the related data in a struct and use one mapping. For instance, for the PuttyV2.sol mappings, the mitigation could be:

struct Position {
  uint256 expiration;
  bool exercised;
  uint256[] floorAssetTokenId;
}

And it would be used as a state variable:

mapping(uint256 => Position) public positions;

Scientific notation

PROBLEM

For readability, it is best to use scientific notation (e.g 10e4) rather than decimal literals(10_000) or exponentiation(10**4)

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

require(order.duration < 10_000 days, "Duration too long")
feeAmount = (order.strike * fee) / 1000

TOOLS USED

Manual Analysis

MITIGATION

Replace the numbers aforementioned with their scientific notation. Consider also writing replacing two numbers with constant variables

Timelock on setters

PROBLEM

It is good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. Here it is merely a suggestion as setFee already has a very reasonable upper boundary.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

function setBaseURI
function setFee

TOOLS USED

Manual Analysis

MITIGATION

Consider adding a timelock to these setters

TransferOwnership can be 2-steps

PROBLEM

The current ownership transfer process uses OpenZeppelin Ownable contract, where the current owner writes the next owner and the change is immediate. You can consider using 2-step process, where the new owner has to accept the ownership. preventing passing the ownership to an uncontrolled account.

SEVERITY

Non-Critical

TOOLS USED

Manual Analysis

Unused returns

PROBLEM

Adding a return statement when the function defines a named return variable is redundant.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

positionId loops through the array of addresses and delete the address to remove when it finds it. The costs of calling this function may vary drastically depending on the location of the address in the array and how large the array is

TOOLS USED

Manual Analysis

MITIGATION

You don't need to explicitly return positionId.

Gas Report

Table of Contents

• Array length should not be looked up in every iteration
• Calldata instead of memory for RO function parameters
• Constant expressions
• Constants can be private
• Constructor parameters are expensive
• Custom errors
• Default value initialization
• Immutable instead of constant for expressions
• Modifier instead of duplicate require
• Prefix increments
• Transfers should be avoided if amount null
• unchecked arithmetic

Array length should not be looked up in every iteration

IMPACT

It wastes gas to read an array's length in every iteration of a for loop, even if it is a memory or calldata array: 3 gas per read.

PROOF OF CONCEPT

10 instances:

PuttyV2.sol

for (uint256 i = 0; i < orders.length; i++)
for (uint256 i = 0; i < assets.length; i++)
for (uint256 i = 0; i < assets.length; i++)
for (uint256 i = 0; i < floorTokens.length; i++)
for (uint256 i = 0; i < assets.length; i++)
for (uint256 i = 0; i < assets.length; i++)
for (uint256 i = 0; i < floorTokens.length; i++)
for (uint256 i = 0; i < whitelist.length; i++)
for (uint256 i = 0; i < arr.length; i++)
for (uint256 i = 0; i < arr.length; i++)

TOOLS USED

Manual Analysis

MITIGATION

Caching the lengths in a variable before the for loop

Calldata instead of memory for RO function parameters

PROBLEM

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory,but it alleviates the compiler from the abi.decode() step that copies each index of the calldata to the memory index, each iteration costing 60 gas.

PROOF OF CONCEPT

Instances:

PuttyV2.sol

scope: fillOrder()

Order memory order
uint256[] memory floorAssetTokenIds

scope: exercise()

Order memory order

scope: exercise()

Order memory order

scope: withdraw()

Order memory order

scope: cancel()

Order memory order

scope: batchFillOrder()

Order memory order
uint256[] memory floorAssetTokenIds

scope: batchFillOrder()

Order memory order
Order memory originalOrder

scope: _transferERC20sIn()

ERC20Asset[] memory assets

scope: _transferERC721sIn()

ERC20Asset[] memory assets

scope: _transferFloorsIn()

address[] memory floorTokens, uint256[] memory floorTokenIds

scope: _transferERC20sOut()

ERC20Asset[] memory assets

scope: _transferERC721sOut()

ERC20Asset[] memory assets

scope: _transferFloorsOut()

address[] memory floorTokens, uint256[] memory floorTokenIds

scope: isWhitelisted()

address[] memory whitelist

scope: hashOppositeOrder()

Order memory order

scope: encodeERC20Assets()

ERC20Asset[] memory arr

scope: encodeERC721Assets()

ERC20Asset[] memory arr

TOOLS USED

Manual Analysis

MITIGATION

Replace memory with calldata

Constant expressions

IMPACT

Constant expressions are re-calculated each time they are in use, costing an extra 97 gas than a constant every time they are called.

PROOF OF CONCEPT

3 instances:

PuttyV2.sol

89:     bytes32 public constant ERC721ASSET_TYPE_HASH =
90:         keccak256(abi.encodePacked("ERC721Asset(address token,uint256 tokenId)"));

95:     bytes32 public constant ERC20ASSET_TYPE_HASH =
96:         keccak256(abi.encodePacked("ERC20Asset(address token,uint256 tokenAmount)"));

101:     bytes32 public constant ORDER_TYPE_HASH =
102:         keccak256(
103:             abi.encodePacked(
104:                 "Order(",
105:                 "address maker,",
106:                 "bool isCall,",
107:                 "bool isLong,",
108:                 "address baseAsset,",
109:                 "uint256 strike,",
110:                 "uint256 premium,",
111:                 "uint256 duration,",
112:                 "uint256 expiration,",
113:                 "uint256 nonce,",
114:                 "address[] whitelist,",
115:                 "address[] floorTokens,",
116:                 "ERC20Asset[] erc20Assets,",
117:                 "ERC721Asset[] erc721Assets",
118:                 ")",
119:                 "ERC20Asset(address token,uint256 tokenAmount)",
120:                 "ERC721Asset(address token,uint256 tokenId)"
121:             )
122:         );

TOOLS USED

Manual Analysis

MITIGATION

Mark these as immutable instead of constant

Constants can be private

IMPACT

Marking constants as private can save a significant amount gas upon deployment, as the compiler does not have to create getter functions for these variables. It is worth noting that a private variable can still be read using either the verified contract source code or the bytecode.

PROOF OF CONCEPT

Instances:

PuttyV2.sol

address public immutable weth

TOOLS USED

Manual Analysis

MITIGATION

Make weth private instead of public,

• deployment cost before:

╭────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮ │ PuttyV2 contract ┆ ┆ ┆ ┆ ┆ │ ╞════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡ │ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤ │ 4774098 ┆ 25226 ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

• deployment cost after:

╭────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮ │ PuttyV2 contract ┆ ┆ ┆ ┆ ┆ │ ╞════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡ │ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤ │ 4761466 ┆ 25156 ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

This change saves 12,632 gas upon deployment.

Constructor parameters are expensive

IMPACT

Constructor parameters are expensive. The contract deployment will be cheaper in gas if they are hard coded instead of using constructor parameters.

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

setBaseURI(_baseURI)
setFee(_fee)
weth = _weth

TOOLS USED

Manual Analysis

MITIGATION

Hardcode weth with its value instead of writing it during deployment with a constructor parameter. Do the same with baseUri and fee, which will also allow you to declare setBaseURI() and setFee() as external instead of public - which does not save gas but is still a good practice for functions not called by the contract itself.

Custom Errors

IMPACT

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here.

It not only saves gas upon deployment - ~9500 gas saved per custom error instead of a require statement, but it is also cheaper in a function call, 22 gas saved per require statement replaced with a custom error.

Custom errors are defined using the error statement

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

require(_weth != address(0), "Unset weth address")
require(_fee < 30, "fee must be less than 3%")
require(SignatureChecker.isValidSignatureNow(order.maker, orderHash, signature), "Invalid signature")
require(!cancelledOrders[orderHash], "Order has been cancelled")
require(order.whitelist.length == 0 || isWhitelisted(order.whitelist, msg.sender), "Not whitelisted")
require(order.duration < 10_000 days, "Duration too long")
require(block.timestamp < order.expiration, "Order has expired")
require(order.baseAsset.code.length > 0, "baseAsset is not contract")
require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")
require(floorAssetTokenIds.length == 0, "Invalid floor tokens length")
require(msg.value == order.premium, "Incorrect ETH amount sent")
require(msg.value == order.strike, "Incorrect ETH amount sent")
require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner")
require(order.isLong, "Can only exercise long positions")
require(block.timestamp < positionExpirations[uint256(orderHash)], "Position has expired")
require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")
require(floorAssetTokenIds.length == 0, "Invalid floor tokens length")
require(msg.value == order.strike, "Incorrect ETH amount sent")
require(!order.isLong, "Must be short position")
require(ownerOf(uint256(orderHash)) == msg.sender, "Not owner")
require(block.timestamp > positionExpirations[longPositionId] || isExercised, "Must be exercised or expired")
require(msg.sender == order.maker, "Not your order")
require(orders.length == signatures.length, "Length mismatch in input")
require(signatures.length == floorAssetTokenIds.length, "Length mismatch in input")
require(token.code.length > 0, "ERC20: Token is not contract")
require(tokenAmount > 0, "ERC20: Amount too small")
require(_ownerOf[id] != address(0), "URI query for NOT_MINTED token")

PuttyV2Nft.sol

require(to != address(0), "INVALID_RECIPIENT")
require(_ownerOf[id] == address(0), "ALREADY_MINTED")
require(from == _ownerOf[id], "WRONG_FROM")
require(to != address(0), "INVALID_RECIPIENT")
require(msg.sender == from || isApprovedForAll[from][msg.sender] || msg.sender == getApproved[id],"NOT_AUTHORIZED")
require(owner != address(0), "ZERO_ADDRESS")

TOOLS USED

Manual Analysis

MITIGATION

Replace require and revert statements with custom errors.

For instance, in PuttyV2.sol:

Replace

- require(tokenAmount > 0, "ERC20: Amount too small")

with

+ if (tokenAmount == 0) {
+		revert AmountZero();
+}

and define the custom error in the contract

+ error AmountZero();

The example above (ie only one require statement replaced with a custom error) saves 9609 gas on deployment:

BEFORE

╭────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮ │ PuttyV2 contract ┆ ┆ ┆ ┆ ┆ │ ╞════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡ │ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤ │ 4774098 ┆ 25226 ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

AFTER

╭────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮ │ PuttyV2 contract ┆ ┆ ┆ ┆ ┆ │ ╞════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡ │ Deployment Cost ┆ Deployment Size ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤ │ 4764489 ┆ 25178 ┆ ┆ ┆ ┆ │ ├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

Default value initialization

IMPACT

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes 3 gas per variable initialized.

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

uint256 feeAmount = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0
uint256 i = 0

TOOLS USED

Manual Analysis

MITIGATION

Remove explicit initialization for default values.

Immutable instead of constant for expressions

IMPACT

Constant expressions are left as expressions, not constants, and are re-calculated each time they are used, costing approximately an extra 100 gas on each access.

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

bytes32 public constant ERC721ASSET_TYPE_HASH = keccak256(abi.encodePacked("ERC721Asset(address token,uint256 tokenId)"))
bytes32 public constant ERC20ASSET_TYPE_HASH = keccak256(abi.encodePacked("ERC20Asset(address token,uint256 tokenAmount)"))
bytes32 public constant ORDER_TYPE_HASH

TOOLS USED

Manual Analysis

MITIGATION

Replace constant with immutable

Modifier instead of duplicate require

PROBLEM

When a require statement is use multiple times, it is cheaper to use a modifier instead.

PROOF OF CONCEPT

4 instances of require statements repeated twice each:

PuttyV2.sol

require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")
require(floorAssetTokenIds.length == order.floorTokens.length, "Wrong amount of floor tokenIds")

require(floorAssetTokenIds.length == 0, "Invalid floor tokens length")
require(floorAssetTokenIds.length == 0, "Invalid floor tokens length")

require(msg.value == order.strike, "Incorrect ETH amount sent")
require(msg.value == order.strike, "Incorrect ETH amount sent")

PuttyV2Nft.sol

require(to != address(0), "INVALID_RECIPIENT")
require(to != address(0), "INVALID_RECIPIENT")

TOOLS USED

Manual Analysis

MITIGATION

Use modifiers for these repeated statements

Prefix increments

IMPACT

Prefix increments are cheaper than postfix increments: it returns the incremented variable instead of returning a temporary variable storing the initial value of the variable. It saves 5 gas per iteration

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

i++
i++
i++
i++
i++
i++
i++
i++
i++
i++

TOOLS USED

Manual Analysis

MITIGATION

change variable++ to ++variable.

Transfers should be avoided if amount null

IMPACT

Gas can be saved by avoiding ERC20.transfer function calls when the amount to be transferred is 0. Merely an observation here as users are not likely to create orders with null order.premium or order.strike

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

• ERC20(order.baseAsset).safeTransferFrom(order.maker, msg.sender, order.premium), ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.maker, order.premium) there is no check that order.premium is not zero
• ERC20(order.baseAsset).safeTransferFrom(order.maker, address(this), order.strike), ERC20(order.baseAsset).safeTransferFrom(msg.sender, address(this), order.strike) there is no check that order.strike is not zero
• ERC20(order.baseAsset).safeTransferFrom(order.maker, address(this), order.strike), ERC20(order.baseAsset).safeTransferFrom(msg.sender, order.strike) there is no check that order.strike is not zero

TOOLS USED

Manual Analysis

MITIGATION

Add checks to ensure the amounts transferred are not 0. Alternatively, this can be handled on the front-end side.

Unchecked arithmetic

IMPACT

The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.

if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas

PROOF OF CONCEPT

Instances include:

PuttyV2.sol

ERC20(order.baseAsset).safeTransfer(msg.sender, order.strike - feeAmount): feeAmount = (order.strike * fee) / 1000, and as _fee < 30, feeAmount < order.strike, so order.strike - feeAmount cannot underflow.

10 instances of an incremented variable in a for loop that cannot overflow as it is bounded by the gas limit:

i++
i++
i++
i++
i++
i++
i++
i++
i++
i++

TOOLS USED

Manual Analysis

MITIGATION

Place the arithmetic operations in an unchecked block