Forgotten Runes Warrior Guild contest - 0xf15ers's results

1. Unhanded return value on tranfser()

• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsGuild.sol#L175

• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L629

• The return value of some implementations of transfer on some tokens returns false instead of reverting.

• It is recommended to check the return value and revert on false or use OpenZeppelin’s SafeERC20 wrapper functions.

2. No Index bounds checks before iterating on array daMinters of issueRefunds()

• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L355
• When the index is out of bound in the array, the transaction is reverted. It is safe to check the index are within the bounds before iteration to prevent unintended reverts.
• Maybe require(startIdx > 0 && endIdx > daMinters.length ) -1 could be used.

1. Checking zero address on msg.sender may-not be required

• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L628
• forwardERC20s() checks for msg.sender != address(0) . It can be removed as it's highly unlikely to have private key of zero address.

2. ++i is cheaper than i++ in loops

• prefix increment is cheaper than postfix increment.
• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L162
• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L220
• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L259
• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L162

3. ++numClaimed and ++numSold is more gas efficient than numClaimed+=1 and numSold+=1

• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L154
• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L193

4. Calculations in refundOwed() can be simplified for more gas efficiency.

• https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L390-L391
• using return daAmountPaid[minter] - (finalPrice * daNumMinted[minter]) - daAmountRefunded[minter] is more gas efficient

//before
|  ForgottenRunesWarriorsMinter  ·  selfRefund               ·          -  ·          -  ·      62257  ·            2  ·          -  │

//After
|  ForgottenRunesWarriorsMinter  ·  selfRefund               ·          -  ·          -  ·      62227  ·            2  ·          -  │

• same optimisation can be applied to currentDaPrice() calculations too.