Ambiguity in the name vaultImplementation

The term vaultImplementation refers to both the beacon and the beacon’s source implementation, creating ambiguity. This confusion can lead to operational errors if the owner mistakes the beacon address stored in _vaultConfig.vaultImplementation for the vault’s source-implementation contract address. For instance, when updating fields in the VaultConfig _vaultConfig storage, other than _vaultConfig.vaultImplementation, the owner might

• read the current VaultConfig struct using factory.vaultConfig()

• modify fields in the VaultConfig instance but leave _vaultConfig.vaultImplementation unchanged, so it still contains the UpgradeableBeacon address

• call factory.updateVaultConfig() with the modified VaultConfig struct

Inside the updateVaultConfig function, the line UpgradeableBeacon(previousVaultImplementation).upgradeTo(config.vaultImplementation) instructs the UpgradeableBeacon contract to upgrade its implementation address to its own address. This action would disrupt the functionality of all vaults that depend on this beacon.

Consider using distinct struct definitions for storage and function parameters to prevent this confusion. For example, define a VaultConfig struct for storage and a VaultConfigParams struct as input for the updateVaultConfig() function. In the VaultConfig storage struct, rename the field to vaultBeacon. Keep the name vaultImplementation in the VaultConfigParams input struct to clearly indicate it expects the implementation address.

DexFi provided the following response to this issue:

In case the owner confuses and sets an invalid address for vaultImplementation, this will only result in the inability of the vaults to operate, which can be fixed by replacing vaultImplementation with the correct implementation.