From 02e700ba091b62f621e91e8bc872f9da4526dcb8 Mon Sep 17 00:00:00 2001 From: Korrrba Date: Sat, 24 Feb 2024 17:39:15 +0100 Subject: [PATCH 1/2] feat: prevent duplicate collateral tokens Resolves: https://github.com/sherlock-audit/2023-12-ubiquity-judging/issues/145 --- .../src/dollar/libraries/LibUbiquityPool.sol | 28 +++++++++++++++++-- .../diamond/facets/UbiquityPoolFacet.t.sol | 13 +++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol b/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol index 2f2a90a35..c5f68dc17 100644 --- a/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol +++ b/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol @@ -202,6 +202,24 @@ library LibUbiquityPool { return poolStorage.collateralAddresses; } + /** + * @notice Check if collateral token with given address already exists + * @param collateralAddress The collateral token address to check + */ + function collateralExists( + address collateralAddress + ) internal view returns (bool) { + UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); + address[] memory collateralAddresses = poolStorage.collateralAddresses; + + for (uint256 i = 0; i < collateralAddresses.length; i++) { + if (collateralAddresses[i] == collateralAddress) { + return true; + } + } + return false; + } + /** * @notice Returns collateral information * @param collateralAddress Address of the collateral token @@ -533,10 +551,9 @@ library LibUbiquityPool { // roundId int256 answer, // startedAt , - uint256 updatedAt, + uint256 updatedAt, // answeredInRound - ) = // answeredInRound - priceFeed.latestRoundData(); + ) = priceFeed.latestRoundData(); // fetch number of decimals in chainlink feed uint256 priceFeedDecimals = priceFeed.decimals(); @@ -631,6 +648,11 @@ library LibUbiquityPool { address chainLinkPriceFeedAddress, uint256 poolCeiling ) internal { + require( + !collateralExists(collateralAddress), + "Collateral already added" + ); + UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); uint256 collateralIndex = poolStorage.collateralAddresses.length; diff --git a/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol b/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol index de8946d67..4124fa947 100644 --- a/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol +++ b/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol @@ -692,6 +692,19 @@ contract UbiquityPoolFacetTest is DiamondTestSetup { assertEq(info.redemptionFee, 20000); } + function testAddCollateralToken_ShouldRevertIfCollateralAlreadyExists() + public + { + uint256 poolCeiling = 50_000e18; + vm.startPrank(admin); + vm.expectRevert("Collateral already added"); + ubiquityPoolFacet.addCollateralToken( + address(collateralToken), + address(collateralTokenPriceFeed), + poolCeiling + ); + } + function testRemoveAmoMinter_ShouldRemoveAmoMinter() public { vm.startPrank(admin); From e7c60c7e09f300cb01675b53adfa81e7a4862aec Mon Sep 17 00:00:00 2001 From: Korrrba Date: Sat, 24 Feb 2024 17:52:24 +0100 Subject: [PATCH 2/2] chore: shorten test name to testAddCollateralToken_ShouldRevertIfCollateralExists As discussed during pull request review. --- .../contracts/test/diamond/facets/UbiquityPoolFacet.t.sol | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol b/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol index 4124fa947..31d17d85c 100644 --- a/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol +++ b/packages/contracts/test/diamond/facets/UbiquityPoolFacet.t.sol @@ -692,9 +692,7 @@ contract UbiquityPoolFacetTest is DiamondTestSetup { assertEq(info.redemptionFee, 20000); } - function testAddCollateralToken_ShouldRevertIfCollateralAlreadyExists() - public - { + function testAddCollateralToken_ShouldRevertIfCollateralExists() public { uint256 poolCeiling = 50_000e18; vm.startPrank(admin); vm.expectRevert("Collateral already added");