Forgotten Runes Warrior Guild contest - saian's results

1. Revert string greater than 32 bytes

Revert strings greater than 32 bytes will consume more gas during deployment and when the require condition is met

Proof of concept

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

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

Mitigation

reduce the string size to 32 bytes

_msgSender() can be replaced with msg.sender

When there is no implementation of meta-transactions, msg.sender can be used directly

Proof of concept

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

Mitigation

replace _msgSender() with msg.sender

Unnecessary address(0) check in ForgottenRunesWarriorsGuild#forwardERC20s

require condition will always be true as msg.sender will not be address(0)

Proof of concept

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

    function forwardERC20s(IERC20 token, uint256 amount) public onlyOwner {
        require(address(msg.sender) != address(0)); 

Mitigation

require statement can be removed

Storage reads can be reduced to save gas

Storage variables that are re-read can be stored in a temporary variables and re-used instead of re-reading to save gas

Proof of concept

tokenId can be used in the first require statement instead of reading from storage

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

    require(numMinted < MAX_WARRIORS, 'All warriors have been summoned');
    require(_msgSender() == minter, 'Not a minter'); 
    uint256 tokenId = numMinted;

numSold, maxDaSupply in

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

numSold, maxForSale in

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

startPrice, lowestPrice, daStartTime,lowestPrice

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

weth in

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

Mitigation

Storage variable can be stored in a temporary variable and re-used

i++ can be replaced with ++i

As return value of ++i is not used, it can be replaced with ++i to save gas, and unchecked can be added as there is no chance of overflow due to the condition

Proof of concept

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#L355

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

Mitigation

post-increment can be replaced with pre-increment

Add unchecked to save gas

Adding unchecked to statements that cant overflow/underflow can avoid the default overflow/underflow checks introduced in v0.8 and save gas

Proof of concept

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

        if (stepDeduction > startPrice) {
            return lowestPrice;                 
        }
        uint256 currentPrice = startPrice - stepDeduction;

Mitigation

Add unchecked to the statement