Forgotten Runes Warrior Guild contest - 0xliumin's results

Judge has assessed an item in Issue #117 as Medium risk. The relevant finding follows:

Don't use send / transfer to send ETH

This is low severity but you should avoid these and use call

Don't use send / transfer to send ETH

This is low severity but you should avoid these and use call

Forward less gas in call

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

An attacker could potentially intentionally use up all the gas forwarded then fail, and still get all the benefit of getting WETH. This is pretty unlikely all things considered, but you could limit the gas forwarded to 10_000 and you'd still be fine.

Users can self refund while contract is paused

This probably doesn't cause any issues but it's possible for users to self refund while the contract is paused. I think it's worth preventing this in weird edge cases where the contract gets into a bad state and you have to manually intervene anyways.

Use one decreasing counter for warriors

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

Here you can use one counter, warriorsLeft, for example, and decrease it for every token minted.

Don't set variables to their default type values

It's unnecessary to set variables to their default values

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

Redundant require statements

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

The second require statement will always be false if the first one is. You can avoid having to call both.

Use custom errors

Using custom errors with revert can also use less gas.

Cache daPriceCurveLength, daStartTime, and daDropINterval

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

You can cache and store the value for thee variables in the currentDaPrice and it will avoid loading them from storage multiple times.

You can also compute elapsed before the if block on line 279 and compare it to the length directly in the if block.

Remove if block on line 292

You can remove the if block because you will never reach that if block if the if block on 279 is false.

Use decreasing counter for numClaimed

You can avoid loading in two variables by just keeping track of the numUnclaimed as opposed to the numClaimed.

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

Make functions external

You can make some functions external to save some gas. This is mostly important for the issueRefunds, which you say you'll be calling a lot. https://github.com/code-423n4/2022-05-runes/blob/060b4f82b79c8308fe65674a39a07c44fa586cd3/contracts/ForgottenRunesWarriorsMinter.sol#L284

Use unchecked arithmetic

You can use unchecked arithmetic in some of the for loops. This can especially save you gas in the issueRefunds call.

Get rid of daAmountRefunded

You can just use subtract the amount owed from daAmountPaid to save gas and not need to store or load the amount refunded.