Nouns Builder contest - _Adam's results

[L01] Transfer Can Fail Silently

Description: The transfer in handleOutgoingTransfer can fail silently and cause a loss of funds to user. It is however an unlikely scenario as the reciever has to be a contract account that fails to recieve the call() transfer and has not implemented a way to deal with ERC20 tokens.

Recommendation: It can't be fixed by changing to safeTransfer as this will open the potential for a user to DoS anyone else who tries to call createBid, so I recommend changing to a pull pattern where the users balance is stored in a variable and they must call a second function to withdraw the funds where safeTransfer can be used.

LOC: Auction.sol#L350-L364

[N01] Incomplete Natspec

Governor.sol#L247 - Missing @param reason

[G01] x = x + y is Cheaper than x += y

Based on test in remix you can save ~1,007 gas on deployment and ~15 gas on execution cost if you use x = x + y over x += y (Is only true for Storage Variables).

contract Test {
	uint256 x = 1;
	function test() external {
		x += 3; 
		(Deployment Cost: 153,124, Execution Cost: 30,369)
		vs
		x = x + 1;
		(Deployment Cost: 152,117, Execution Cost: 30,354)
	}

}

Instances where x = x + y/ x = x - y can be used: Governor.sol#L280 Governor.sol#L285 Governor.sol#L290

[G02] Packing Variables in Structs

Both ownershipPct & vestExpiry are cast down to uints < 256 when they are used throughout the contracts, so we can safely change both to uint128 and condense into 1 storage slot.

IManager.sol#L50-L51