code-423n4 / 2021-11-badgerzaps-findings

0 stars 0 forks source link

Adding `recipient` parameter to mint functions can help avoid unnecessary token transfers and save gas #34

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

WatchPug

Vulnerability details

Given the fact that the zap contracts will be layered, one mint() function may call another mint() function and tokens will be transferred multiple times from contract to contract. If we can add a recipient parameter, the unnecessary contract to contract token transfers can be avoided and save gas.

For example:

SettToRenIbbtcZap.sol#mint() will call IBBTC_MINT_ZAP.mint(), and IBBTC_MINT_ZAP will mint and transfer the minted tokens back to SettToRenIbbtcZap contract, and it will then send to the user (msg.sender)

https://github.com/Badger-Finance/badger-ibbtc-utility-zaps/blob/8d265aacb905d30bd95dcd54505fb26dc1f9b0b6/contracts/SettToRenIbbtcZap.sol#L311-L322

    // Use other zap to deposit
    uint256 ibbtcAmount = IBBTC_MINT_ZAP.mint(
        address(zapConfig.withdrawToken),
        btcAmount,
        0, // poolId - renCrv: 0
        address(zapConfig.withdrawToken) == address(RENBTC) ? 0 : 1, // idx - renbtc: 0, wbtc: 1
        _minOut
    );
    IBBTC.safeTransfer(msg.sender, ibbtcAmount);

    return ibbtcAmount;
}

If a recipient parameter get added to IBBTC_MINT_ZAP.mint(), the above codes can be changed to:

    // Use other zap to deposit
    return IBBTC_MINT_ZAP.mint(
        address(zapConfig.withdrawToken),
        btcAmount,
        0, // poolId - renCrv: 0
        address(zapConfig.withdrawToken) == address(RENBTC) ? 0 : 1, // idx - renbtc: 0, wbtc: 1
        _minOut,
        msg.sender // recipient
    );
}
GalloDaSballo commented 3 years ago

I appreciate the advice, i think there would be some unintended consequences, but will think about it, thanks!