#19 Consider implementing two-step procedure for updating protocol addresses

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultManagerV5.sol#L115

Summary

Consider implementing two-step procedure for updating protocol addresses

Vulnerability Details

A copy-paste error or a typo may end up bricking protocol functionality, or sending tokens to an address with no known private key. Consider implementing a two-step procedure for updating protocol addresses, where the recipient is set as pending, and must "accept" the assignment by making an affirmative call. A straight forward way of doing this would be to have the target contracts implement EIP-165, and to have the "set" functions ensure that the recipient is of the right interface type.

File: contracts/SmartVaultManagerV5.sol

/// @audit `weth`
115:     function setWethAddress(address _weth) external onlyOwner() {

/// @audit `swapRouter2`
119:     function setSwapRouter2(address _swapRouter) external onlyOwner() {

/// @audit `nftMetadataGenerator`
123:     function setNFTMetadataGenerator(address _nftMetadataGenerator) external onlyOwner() {

/// @audit `smartVaultDeployer`
127:     function setSmartVaultDeployer(address _smartVaultDeployer) external onlyOwner() {

/// @audit `protocol`
131:     function setProtocolAddress(address _protocol) external onlyOwner() {

/// @audit `liquidator`
135:     function setLiquidatorAddress(address _liquidator) external onlyOwner() {

Github: [115, 119, 123, 127, 131, 135]

File: contracts/SmartVaultV3.sol

/// @audit `owner`
233:     function setOwner(address _newOwner) external onlyVaultManager {

Github: [233]

Impact

See Vulnerability Details

Tools Used

Manual Review

Recommendations

Consider two-step procedure for updating protocol addresses.