Putty contest - samruna's results

QA report

1. Return statement and parameterized return

Code: https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L268

Description: Above code used both parameterized return and a return statement. Since this function is only returning one value, no need to do return twice.

return positionId

Mitigation: Only keep one return

2. Use of != 0 instead of > 0 for uint parameters

Code: https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L427 https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L293 https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L327 https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L351 https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L498 https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L598-599

Mitigation: Checking for !=0 saves some gas.

3. Event emitted before transaction is complete

Code: https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L318

Description: In above function, event FilledOrder() is emitted before the transaction is complete. If the purpose of event is just to inform about the order, then this is fine. But if the purpose if leave trace about entire transaction, then this should be last statement in the function.

Mitigation: Please emit only after the transaction is complete.

4. Code can be pulled into separate function

Code: https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L323-378

Description: Above code can be extracted into a separate function for readaility. Also the conditions can be arranged in much better way for anyone to understand the flow.

Mitigation: Add another internal function processOrder(order)

function processOrder(Order memory order) internal returns (uint256 positionId) { bool isCall = order.isCall; bool isLong = order.isLong

if (isCall) {
	.
	.
	.
	if (!isCall) {
		.
		.
		return positionId=...
	} else (isCall) {
		.
		.
		return positionId=...
	}
}

} repeat same for isLong

This will help the readaility, flow and may be make the contract better arranged.

5. Use of modified for repeated validations Code: https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L395 https://github.com/code-423n4/2022-06-putty/blob/main/contracts/src/PuttyV2.sol#L475

Above two code lines are similar checks. They can be turned into modifiers for readability