Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect parameter name in the create function could lead to initialization mistakes #220

Closed
tuturu-tech opened this issue Oct 18, 2023 · 0 comments
Assignees

Comments

@tuturu-tech
Copy link
Collaborator

Severity: Informational
Difficulty: High
Type: Data Validation
Target: ve-silo/contracts/guages/ethereum/LiquidityGaugeFactory.sol, ve-silo/contracts/guages/l2-common/ChildChainGaugeFactory.sol

Description
The incorrect parameter name in the create function of the LiquidityGaugeFactory contract can lead to the wrong configuration of the contract, which will require re-deployment of the contract to correct.

The SiloLiquidityGauge contract has been refactored to use an ERC20 token receiver hook instead of using an ERC20BalanceHandler to automate updates of the user checkpoints and liquidity along with the transfer of the Silo liquidity tokens. However, the factory contracts for the liquidity gauge contracts have not been updated to reflect this change. Specifically, the create function of the LiquidityGaugeFactory contract uses the parameter name erc20BalancesHandler for the hook_receiver parameter:

function create(uint256 relativeWeightCap, address erc20BalancesHandler) external returns (address) {
    address gauge = _create();
    ISiloLiquidityGauge(gauge).initialize(relativeWeightCap, erc20BalancesHandler);
    return gauge;
}

The wrong parameter name can cause confusion and result in the wrong initialization of the liquidity gauge contracts created with the factory contract. The wrong initialization can not be corrected without creating another gauge for the same pool.

The issue also affects the following contracts:

  1. The create function of the ILiquidityGaugeFactory interface contract
  2. The initialize function of the ISiloLiquidityGauge interface contract
  3. The create function of the ChildChainGaugeFactory contract
  4. The create function of the IChildChainGaugeFactory interface contract
  5. The initialize function of the ISiloChildChainGauge interface contract

Recommendations
Short term, rename the erc20BalanceHandler parameter in all the functions to the hook_receiver parameter.

Long term, review the whole codebase during refactors to ensure correct refactoring of all the contracts interacting with each other.

@tuturu-tech tuturu-tech changed the title Incorrect parameter name in the create function could lead to initialization mistakes Incorrect parameter name in the create function could lead to initialization mistakes Oct 18, 2023
IhorSF added a commit that referenced this issue Nov 14, 2023
@IhorSF IhorSF self-assigned this Dec 1, 2023
@IhorSF IhorSF closed this as completed Dec 15, 2023
techsaavy8784 pushed a commit to techsaavy8784/silo-contracts-v2 that referenced this issue Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants