# Giddy Contract Review #2

**GiddyVaultV2.sol**

Note: This function seems like only a liability? You dont remove the funds from the old strategy so setting strategy only seems to erase all deposits?

function setStrategy(address strategyAddress) public onlyOwner {
    strategy = GiddyStrategyV2(strategyAddress);
  }

**[H:01] Withdraw doesnt account for situation where LP is emergency withdrawn.**

If you need to remove the funds from the chef via emergencyWithdraw() the funds will be stuck in the strategy since withdraw function always tries to withdraw from the chef. To clarify, though you may be able to withdraw 0. emergencyWithdraw() could have been used because withdraw() was failing, calling withdraw when there is nothing to withdraw could put you at risk.

**[M:01] Signatures may not match**

In function validateVaultAuth. The bytes memory dataArray variable is overwritten everytime in two consecutive for loops. This would leave the result as the last write of the dataArray variable. That might not match how its built on the app side in the origin signature. Code snippet:

function validateVaultAuth(VaultAuth calldata auth) private {
    require(block.timestamp < auth.deadline, "SWAP_AUTH_EXPRIED");
    require(!nonceMap[auth.nonce], "NONCE_USED");
    **bytes memory dataArray;**
    for (uint i = 0; i < auth.depositSwaps.length; i++) {
      **dataArray** = abi.encodePacked(dataArray, keccak256(auth.depositSwaps[i].data));
    }
    for (uint i = 0; i < auth.compoundSwaps.length; i++) {
      **dataArray** = abi.encodePacked(dataArray, keccak256(auth.compoundSwaps[i].data));
    }
    bytes memory data = abi.encodePacked(SWAP_AUTHORIZATION_TYPEHASH, abi.encode(
      auth.nonce,
      auth.deadline,
      auth.amount,
      auth.fap,
      auth.fapIndex,
      **keccak256(dataArray)**
    ));
    require(config.verifiedContracts(EIP712.recover(domainSeparator, auth.signature, data)), "VERIFY_SWAP");
    nonceMap[auth.nonce] = true;
  }

**[M:02] Fees can't be deducted**

In depositNative() native is sent directly to strategy yet deductFee tries to transfer fees from the vault to feeAccount(). There is no native in the vault to transfer.

SafeERC20Upgradeable.safeTransferFrom(IERC20Upgradeable(nativeToken), _msgSender(), address(strategy), vaultAuth.amount);

uint256 staked = strategy.depositNative(deductFee(nativeToken, vaultAuth.amount, vaultAuth.fap));

Same issue in depositExact()

SafeERC20Upgradeable.safeTransferFrom(IERC20Upgradeable(depositTokens[i]), _msgSender(), address(strategy), amounts[i]);

if (vaultAuth.fap > 0 && vaultAuth.fapIndex == i) {
          amounts[i] = deductFee(depositTokens[i], amounts[i], vaultAuth.fap);
        }

**[M:03] oneInchSwap in Vault and Strategy doesnt match the interface in GiddyLibrary.**

In vault and example swap is:

GiddyLibraryV2.oneInchSwap(swaps[i], address(this), address(strategy), depositTokens[i]);

In the GiddyLibrary the oneInchSwap function is:

function oneInchSwap(address srcAccount, address dstAccount, address srcToken, address dstToken, uint amount, bytes calldata data) internal returns (uint returnAmount)

**[L:01] Get contract rewards has wrong output for quick**

You query the rewards available in the masterchef which returns amount of dQuick available. This is not equal to quick though getContractBalances( ) returns as if was.

 function getContractRewards() public view override returns (uint256[] memory amounts) {
    amounts = new uint256[](2);
    amounts[0] += IRewarder(IMasterChef(MASTER_CHEF).getRewarder(PID, 1)).pendingToken(PID, address(this));
    amounts[0] += IERC20(WMATIC_TOKEN).balanceOf(address(this));
    amounts[1] += IRewarder(IMasterChef(MASTER_CHEF).getRewarder(PID, 0)).pendingToken(PID, address(this));
    amounts[1] += IERC20(QUICK_TOKEN).balanceOf(address(this));
  }