code-423n4 / 2022-01-yield-findings

1 stars 0 forks source link

Function `ConvexYieldWrapper.removeVault()` can be rewritten #127

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

throttle

Vulnerability details

Impact

Gas savings

Proof of Concept

Function ConvexYieldWrapper.removeVault() can be rewritten. As such, 1 variable can be removed and 1 MLOAD, 1 MSTORE can be saved.

Tools Used

Manual review

Recommended Mitigation Steps

Rewrite this:

function removeVault(bytes12 vaultId, address account) public {
    address owner = cauldron.vaults(vaultId).owner;
    if (account != owner) {
        bytes12[] storage vaults_ = vaults[account];
        uint256 vaultsLength = vaults_.length;
        bool found;
        for (uint256 i = 0; i < vaultsLength; i++) {
            if (vaults_[i] == vaultId) {
                bool isLast = i == vaultsLength - 1;
                if (!isLast) {
                    vaults_[i] = vaults_[vaultsLength - 1];
                }
                vaults_.pop();
                found = true;
                emit VaultRemoved(account, vaultId);
                break;
            }
        }
        require(found, "Vault not found");
        vaults[account] = vaults_;
    }
}

To this

function removeVault(bytes12 vaultId, address account) public {
    address owner = cauldron.vaults(vaultId).owner;
    if (account != owner) {
        bytes12[] storage vaults_ = vaults[account];
        uint256 vaultsLength = vaults_.length;

        for (uint256 i = 0; i < vaultsLength; i++) {
            if (vaults_[i] == vaultId) {
                bool isLast = i == vaultsLength - 1;
                if (!isLast) {
                    vaults_[i] = vaults_[vaultsLength - 1];
                }
                vaults_.pop();
                vaults[account] = vaults_;

                emit VaultRemoved(account, vaultId);
                return;
            }
        }
        revert("Vault not found");
    }
}
alcueca commented 2 years ago

Duplicate #111