Al-Qa-qa / dyad-private-audit

4 stars 1 forks source link

[M-03] `DeployVaultManagerV3` compilation will fail as `encodeCall()` expects two args #11

Open Al-Qa-qa opened 4 months ago

Al-Qa-qa commented 4 months ago

This issue introduced when mitigating H-02 issue on the Private Review.

Description

When mitigating the issue, the team chooses to not reset the variables again. But the problem is that they let encodeCall() function takes only one argument, which is the function signature, and not the arguments.

    Upgrades.upgradeProxy(
      MAINNET_V2_VAULT_MANAGER,
      "VaultManagerV3.sol",
@>    abi.encodeCall(VaultManagerV3.initialize)
    );

The problem here is that encodeCall() takes only one argument, and it should takes two. The first for functionPointer, and the second is a tuble with args.

https://docs.soliditylang.org/en/latest/cheatsheet.html#abi-encoding-and-decoding-functions

abi.encodeCall(function functionPointer, (...)) returns (bytes memory): ABI-encodes a call to functionPointer with the arguments found in the tuple. Performs a full type-check, ensuring the types match the function signature. Result equals abi.encodeWithSelector(functionPointer.selector, ...)

This will result in a compilation process fail, as the function should take two args, and the passed is just one.

Recommended Mitigation

Pass an empty tuble in the second arg.

    Upgrades.upgradeProxy(
      MAINNET_V2_VAULT_MANAGER,
      "VaultManagerV3.sol",
-     abi.encodeCall(VaultManagerV3.initialize)
+     abi.encodeCall(VaultManagerV3.initialize, ())

    );
shafu0x commented 4 months ago

should be fixed now: https://github.com/DyadStablecoin/contracts/pull/52/files

Al-Qa-qa commented 4 months ago

The issue has been fixed as recommended.